lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Thu, 10 May 2007 13:56:27 +0530 From: "Satyam Sharma" <satyam.sharma@...il.com> To: "Pallipadi, Venkatesh" <venkatesh.pallipadi@...el.com> Cc: "Jarek Poplawski" <jarkao2@...pl>, "Andrew Morton" <akpm@...ux-foundation.org>, linux-kernel@...r.kernel.org, "Oleg Nesterov" <oleg@...sign.ru> Subject: Re: [PATCH -mm] timer: parenthesis fix in tbase_get_deferrable() etc. > >> >> > static inline unsigned int > >tbase_get_deferrable(tvec_base_t *base) > >> >> > { > >> >> > - return ((unsigned int)(unsigned long)base & > >> >TBASE_DEFERRABLE_FLAG); > >> >> > + return (unsigned int)((unsigned long)base & > >> >TBASE_DEFERRABLE_FLAG); > >> >> > } > >> >... > >> >> The change makes sense, but does it actually "fix" anything? > >> >> > >> > > >> >Yes - this first place fixes logical error, so it's a sin > >> >- even if not punishable in practice. (It's also unnecessary > >> >test for long to int conversion.) > >> > > >> > >> I am sorry, I don't understand. What is the logical error in > >the first > >> one? > >> > >> Actually, your change makes it different from what was originally > >> indended. > >> Original intention was to type convert base to a 32 bit value and > >> bitwise& with FLAG. > > > >But that is not what the original code is doing. If you wanted to > >typecast "base" to "a 32 bit value" then you should've used u32 > >instead. > > > >Anyway, if you originally intended to actually typecast "base" to > >unsigned int, then you could do that directly without typecasting it > >first to unsigned long (unnecessarily) and then to unsigned int. Of > >course, if your system implements a pointer as something bigger than > >unsigned int (which is what you eventually convert "base" to), then > >you're screwed anyway and the intermediate typecast to unsigned long > >doesn't buy you anything at all. > > On a 64 bit system, converting pointer to int causes unnecessary > compiler > warning, and intermediate long conversion was to avoid that. I will have Whoa! Hello, hold on, just wait a second there. Do you _really_ want an unsigned int return out of tbase_get_deferrable() or will an unsigned long do? If the rest of your code is fine with unsigned long, then I'd suggest something like: static inline unsigned long tbase_get_deferrable(tvec_base_t *base) { return ((unsigned long)base & TBASE_DEFERRABLE_FLAG); } I don't really know your code (so I could be horribly incorrect here), but personally I would prefer *heeding* to that warning than _hiding_ it -- it's not unnecessary, it's telling you that you're *losing* data by converting a pointer (which is always unsigned long) to unsigned int for 64-bit platforms where sizeof(void *) == sizeof(unsigned long) == 8 bytes, but sizeof(unsigned int) == 4. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists