| Message ID | 1395470329-15065-1-git-send-email-behanw@converseincode.com |
|---|---|
| State | New |
| Headers | show |
On Saturday 22 March 2014, behanw@converseincode.com wrote: > diff --git a/include/asm-generic/cmpxchg-local.h b/include/asm-generic/cmpxchg-local.h > index d8d4c89..4c41bb8 100644 > --- a/include/asm-generic/cmpxchg-local.h > +++ b/include/asm-generic/cmpxchg-local.h > @@ -41,6 +41,7 @@ static inline unsigned long __cmpxchg_local_generic(volatile void *ptr, > break; > default: > wrong_size_cmpxchg(ptr); > + prev = 0; > } > raw_local_irq_restore(flags); > return prev; How about using __unreachable() instead to annotate the fact that you won't get here? Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
On 03/22/14 03:01, Arnd Bergmann wrote: > On Saturday 22 March 2014, behanw@converseincode.com wrote: >> diff --git a/include/asm-generic/cmpxchg-local.h b/include/asm-generic/cmpxchg-local.h >> index d8d4c89..4c41bb8 100644 >> --- a/include/asm-generic/cmpxchg-local.h >> +++ b/include/asm-generic/cmpxchg-local.h >> @@ -41,6 +41,7 @@ static inline unsigned long __cmpxchg_local_generic(volatile void *ptr, >> break; >> default: >> wrong_size_cmpxchg(ptr); >> + prev = 0; >> } >> raw_local_irq_restore(flags); >> return prev; > > How about using __unreachable() instead to annotate the fact that you won't > get here? Yours is a _much_ better idea. Will fix. Thanks, Behan
On 03/21/2014 11:38 PM, behanw@converseincode.com wrote: > From: Behan Webster <behanw@converseincode.com> > > Fix uninitialized return code in default case in cmpxchg-local.h > > This patch fixes the code to prevent an uninitialized return value that is detected > when compiling with clang. The bug produces numerous warnings when compiling the > Linux kernel with clang. > > Signed-off-by: Behan Webster <behanw@converseincode.com> > Signed-off-by: Mark Charlebois <charlebm@gmail.com> > --- > include/asm-generic/cmpxchg-local.h | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/include/asm-generic/cmpxchg-local.h b/include/asm-generic/cmpxchg-local.h > index d8d4c89..4c41bb8 100644 > --- a/include/asm-generic/cmpxchg-local.h > +++ b/include/asm-generic/cmpxchg-local.h > @@ -41,6 +41,7 @@ static inline unsigned long __cmpxchg_local_generic(volatile void *ptr, > break; > default: > wrong_size_cmpxchg(ptr); > + prev = 0; > } > raw_local_irq_restore(flags); > return prev; > Shouldn't this be a build time assert (__compiletime_error())? -hpa -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
On 03/31/14 13:52, H. Peter Anvin wrote: > On 03/21/2014 11:38 PM, behanw@converseincode.com wrote: >> From: Behan Webster <behanw@converseincode.com> >> >> Fix uninitialized return code in default case in cmpxchg-local.h >> >> This patch fixes the code to prevent an uninitialized return value that is detected >> when compiling with clang. The bug produces numerous warnings when compiling the >> Linux kernel with clang. >> >> Signed-off-by: Behan Webster <behanw@converseincode.com> >> Signed-off-by: Mark Charlebois <charlebm@gmail.com> >> --- >> include/asm-generic/cmpxchg-local.h | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/include/asm-generic/cmpxchg-local.h b/include/asm-generic/cmpxchg-local.h >> index d8d4c89..4c41bb8 100644 >> --- a/include/asm-generic/cmpxchg-local.h >> +++ b/include/asm-generic/cmpxchg-local.h >> @@ -41,6 +41,7 @@ static inline unsigned long __cmpxchg_local_generic(volatile void *ptr, >> break; >> default: >> wrong_size_cmpxchg(ptr); >> + prev = 0; >> } >> raw_local_irq_restore(flags); >> return prev; >> > Shouldn't this be a build time assert (__compiletime_error())? I changed it to a __noreturn on wrong_size_cmpxchg thanks to James Bottomley. Which would be better? Behan
On 03/31/2014 03:10 PM, Behan Webster wrote: >>> >>> diff --git a/include/asm-generic/cmpxchg-local.h >>> b/include/asm-generic/cmpxchg-local.h >>> index d8d4c89..4c41bb8 100644 >>> --- a/include/asm-generic/cmpxchg-local.h >>> +++ b/include/asm-generic/cmpxchg-local.h >>> @@ -41,6 +41,7 @@ static inline unsigned long >>> __cmpxchg_local_generic(volatile void *ptr, >>> break; >>> default: >>> wrong_size_cmpxchg(ptr); >>> + prev = 0; >>> } >>> raw_local_irq_restore(flags); >>> return prev; >>> >> Shouldn't this be a build time assert (__compiletime_error())? > I changed it to a __noreturn on wrong_size_cmpxchg thanks to James > Bottomley. > > Which would be better? > __compiletime_error traps at compile time rather than link time, which is what you want. -hpa -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
On 03/31/14 15:11, H. Peter Anvin wrote: > On 03/31/2014 03:10 PM, Behan Webster wrote: >>>> diff --git a/include/asm-generic/cmpxchg-local.h >>>> b/include/asm-generic/cmpxchg-local.h >>>> index d8d4c89..4c41bb8 100644 >>>> --- a/include/asm-generic/cmpxchg-local.h >>>> +++ b/include/asm-generic/cmpxchg-local.h >>>> @@ -41,6 +41,7 @@ static inline unsigned long >>>> __cmpxchg_local_generic(volatile void *ptr, >>>> break; >>>> default: >>>> wrong_size_cmpxchg(ptr); >>>> + prev = 0; >>>> } >>>> raw_local_irq_restore(flags); >>>> return prev; >>>> >>> Shouldn't this be a build time assert (__compiletime_error())? >> I changed it to a __noreturn on wrong_size_cmpxchg thanks to James >> Bottomley. >> >> Which would be better? >> > __compiletime_error traps at compile time rather than link time, which > is what you want. The idea is to remove the compile time warning that the return code "prev" isn't initialized in the default case. Indicating that wrong_size_cmpxchg doesn't return fixes that false positive. Behan
On 03/31/2014 03:16 PM, Behan Webster wrote: >>> >> __compiletime_error traps at compile time rather than link time, which >> is what you want. > The idea is to remove the compile time warning that the return code > "prev" isn't initialized in the default case. Indicating that > wrong_size_cmpxchg doesn't return fixes that false positive. > Perhaps __compiletime_error() should have __noreturn built in? -hpa -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
diff --git a/include/asm-generic/cmpxchg-local.h b/include/asm-generic/cmpxchg-local.h index d8d4c89..4c41bb8 100644 --- a/include/asm-generic/cmpxchg-local.h +++ b/include/asm-generic/cmpxchg-local.h @@ -41,6 +41,7 @@ static inline unsigned long __cmpxchg_local_generic(volatile void *ptr, break; default: wrong_size_cmpxchg(ptr); + prev = 0; } raw_local_irq_restore(flags); return prev;