Yeah, this is kind of wrong.
In wondered for a bit why this hadn't ever cause a problem, but
actually, things aren't really broken right now as the seqid
increment a bit down also wraps at 0x3fffff (5 f's) and the
shift overflow is harmless, I think.
But I've queued a fix for this.
Thanks,
-corey
On 11/10/2016 01:35 PM, Dan Carpenter wrote:
> Hi Corey,
>
> The patch 1da177e4c3f4: "Linux-2.6.12-rc2" from Apr 16, 2005, leads
> to the following static checker warning:
>
> drivers/char/ipmi/ipmi_msghandler.c:1760 i_ipmi_request()
> warn: potential shift truncation. '0xff << 26'
>
>
> drivers/char/ipmi/ipmi_msghandler.c
> 157 /*
> 158 * Store the information in a msgid (long) to allow us to find a
> 159 * sequence table entry from the msgid.
> 160 */
> 161 #define STORE_SEQ_IN_MSGID(seq, seqid) (((seq&0xff)<<26) |
> (seqid&0x3ffffff))
>
> The STORE_SEQ_IN_MSGID() and GET_SEQ_FROM_MSGID() don't match.
>
> It seems clear enough that it should be (((seq & 0x3f) << 26) and that
> would silence the static checker warning. But I think also that in
> GET_SEQ_FROM_MSGID() and NEXT_SEQID() we should be using 0x3ffffff
> (with 6 f's instead of 5).
>
> 162
> 163 #define GET_SEQ_FROM_MSGID(msgid, seq, seqid) \
> 164 do {
> \
> 165 seq = ((msgid >> 26) & 0x3f);
> \
> 166 seqid = (msgid & 0x3fffff);
> \
> 167 } while (0)
> 168
> 169 #define NEXT_SEQID(seqid) (((seqid) + 1) & 0x3fffff)
> 170
>
> regards,
> dan carpenter
------------------------------------------------------------------------------
_______________________________________________
Openipmi-developer mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openipmi-developer
