11789 gld: cast between incompatible function types

Review Request #2375 — Created Oct. 9, 2019 and submitted

tsoome
illumos-gate
11789
2589878...
general
../../common/io/gld.c:2904:10: error: cast between incompatible function types from 'int (*)(queue_t *, mblk_t *)' {aka 'int (*)(struct queue *, struct msgb *)'} to 'void (*)(queue_t *, mblk_t *)' {aka 'void (*)(struct queue *, struct msgb *)'} [-Werror=cast-function-type]
   send = (void (*)(queue_t *, mblk_t *))putq;
          ^
cc1: all warnings being treated as errors


citrus
  1. Ship It!
  2. 
      
gdamore
  1. So casting away feels kind of not right here. The issue is that putq and company are defined to return an int, but the functions generally return void. I wonder if we would be better off not casting this at all, but defining the send variable correctly instead. I think it's probably safe the way it is, but only because I think there is an assumption that rax/eax is trashed in the caller, even when calling a void function. (The ABI might actually specify that, I'm not sure.)

    Still making send a variable of type int ()(queue_t , mblk_t *) seems better. (note that queue_t is struct queue and mblk_t is struct msgb, so those types are equivalent.)

    1. Note that RB does weird markup things with asterisks, so the type delcarations above I made don't look quite right. But I think you know what I mean.

    2. The problem there is that putq() and putnext() are returning int and void, so we can not just define send to return int; I did consider to have wrapper for putq - which is rather trivial thing to do, but in this case I figured we might just go for dropping rax because a) this is what is happening anyhow and not just here but all over the kernel and b) by ABI, the RAX is temporary register and first return value register, so it should be safe just to drop the return value. However, I do not mind adding wrapper for putq(), I have used that approach with streams modules before. I actually even like wrapper more TBH:)

    3. I hadn't considered that difference. putq() is really old stuff, so I guess I'm fine with this as is.

  2. 
      
gdamore
  1. Ship It!
  2. 
      
tsoome
Review request changed

Status: Closed (submitted)

Loading...