8232 pcmcia: misleading-indentation

Review Request #515 — Created May 15, 2017 and submitted

tsoome
illumos-gate
8232
a79ae64...
general
../../common/pcmcia/nexus/pcmcia.c: In function 'pcmcia_intr_disable_isr':
../../common/pcmcia/nexus/pcmcia.c:5428:4: error: this 'if' clause does not guard... [-Werror=misleading-indentation]
    if (intr->handler_id != (uint32_t)(uintptr_t)rdip)
    ^~
../../common/pcmcia/nexus/pcmcia.c:5434:5: note: ...this statement, but the latter is misleadingly indented as if it is guarded by the 'if'
     ispecp = (struct intrspec *)&sockp->ls_intrspec;
     ^~~~~~
cc1: all warnings being treated as errors


  • 0
  • 0
  • 2
  • 0
  • 2
Description From Last Updated
tsoome
rm
  1. 
      
  2. I think this makes sense to move. It seems to me that  getting the only handler versus the rdip is assignment. What are your thoughts on this?
    1. After diggig more, in pcmcia_remove_intr_impl() the comments hints that if sockp->ls_inthandlers is set, then its multi function card (MFC), and for MFC, the interrupt specification is in sockp->ls_intrspec. So for MFC we need to use ispecp = (struct intrspec *)&sockp->ls_intrspec; Now the if statement before the ispecp assignment is about picking up the rdip from the socket, but we need to set ispecp from the sockp in any case, so I think the ispecp assignment does not really belong into if body because the MFC intrspec is stored in sockp.

  3. 
      
gwr
  1. 
      
  2. Yeah, as (I think) Robert suggested,
    I think that ispecp assignment was
    meant to be part of the "if" body.

    1. Since this note is about the same issue as Robert was asking, I wrote my opinion next to Robert's question. But to make the code a bit more clear, I did add braces around the if block:) And just as side note, I really have no way to test, but at least as the change does not alter the current behavior, at least this change does not make things worse:)

    2. OK, thanks for looking into this in more detail.
      Update change looks better.

  3. 
      
tsoome
rm
  1. Ship It!
  2. 
      
yuripv
  1. Ship It!
  2. 
      
tsoome
Review request changed

Status: Closed (submitted)

Loading...