| Message ID | 1348874411-28288-6-git-send-email-daniel.santos@pobox.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On Fri, Sep 28, 2012 at 06:20:06PM -0500, Daniel Santos wrote: > __linktime_error() does the same thing as __compiletime_error() and is > only used in bug.h. Since the macro defines a function attribute that > will cause a failure at compile-time (not link-time), it makes more > sense to keep __compiletime_error(), which is also neatly mated with > __compiletime_warning(). > > Signed-off-by: Daniel Santos <daniel.santos@pobox.com> Why not change bug.h in the same commit? Or alternatively, why not change it first? Getting rid of __linktime_error *before* changing its use in bug.h to __compiletime_error seems wrong. - Josh Triplett -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 09/28/2012 07:23 PM, Josh Triplett wrote: > On Fri, Sep 28, 2012 at 06:20:06PM -0500, Daniel Santos wrote: >> __linktime_error() does the same thing as __compiletime_error() and is >> only used in bug.h. Since the macro defines a function attribute that >> will cause a failure at compile-time (not link-time), it makes more >> sense to keep __compiletime_error(), which is also neatly mated with >> __compiletime_warning(). >> >> Signed-off-by: Daniel Santos <daniel.santos@pobox.com> > Why not change bug.h in the same commit? Or alternatively, why not > change it first? I'm still new to this project's development process and wasn't sure if those two changes would be considered lumping multiple changes together or not. So that type of lumping is acceptable then? I certainly wouldn't mind squashing them. > Getting rid of __linktime_error *before* changing its > use in bug.h to __compiletime_error seems wrong. Good point! -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Sep 28, 2012 at 07:34:36PM -0500, Daniel Santos wrote: > On 09/28/2012 07:23 PM, Josh Triplett wrote: > > On Fri, Sep 28, 2012 at 06:20:06PM -0500, Daniel Santos wrote: > >> __linktime_error() does the same thing as __compiletime_error() and is > >> only used in bug.h. Since the macro defines a function attribute that > >> will cause a failure at compile-time (not link-time), it makes more > >> sense to keep __compiletime_error(), which is also neatly mated with > >> __compiletime_warning(). > >> > >> Signed-off-by: Daniel Santos <daniel.santos@pobox.com> > > Why not change bug.h in the same commit? Or alternatively, why not > > change it first? > I'm still new to this project's development process and wasn't sure if > those two changes would be considered lumping multiple changes together > or not. So that type of lumping is acceptable then? I certainly > wouldn't mind squashing them. In general, you shouldn't make *unrelated* changes in the same patch, and you should definitely make fine-grained patches whenever appropriate. However, in this case the changes seem quite related. The rule in the Linux kernel: the kernel should compile and function after every individual patch. You shouldn't introduce breakage in a patch series and fix it later in that series. Commonly called making your patch series "bisectable", since it helps "git bisect" work more smoothly to have every single git commit compile and run. In this case, you removed __linktime_error before removing its caller. > > Getting rid of __linktime_error *before* changing its > > use in bug.h to __compiletime_error seems wrong. > Good point! Thinking about it further, in this case I think it makes sense to just reverse the two patches: remove the one and only use of __linktime_error, then remove its definition. - Josh Triplett -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/include/linux/compiler-gcc4.h b/include/linux/compiler-gcc4.h index b44307d..ad610f2 100644 --- a/include/linux/compiler-gcc4.h +++ b/include/linux/compiler-gcc4.h @@ -33,8 +33,6 @@ the kernel context */ #define __cold __attribute__((__cold__)) -#define __linktime_error(message) __attribute__((__error__(message))) - #ifndef __CHECKER__ # define __compiletime_warning(message) __attribute__((warning(message))) # define __compiletime_error(message) __attribute__((error(message))) diff --git a/include/linux/compiler.h b/include/linux/compiler.h index f430e41..fd455aa 100644 --- a/include/linux/compiler.h +++ b/include/linux/compiler.h @@ -297,9 +297,6 @@ void ftrace_likely_update(struct ftrace_branch_data *f, int val, int expect); #ifndef __compiletime_error # define __compiletime_error(message) #endif -#ifndef __linktime_error -# define __linktime_error(message) -#endif /* * Prevent the compiler from merging or refetching accesses. The compiler * is also forbidden from reordering successive instances of ACCESS_ONCE(),
__linktime_error() does the same thing as __compiletime_error() and is only used in bug.h. Since the macro defines a function attribute that will cause a failure at compile-time (not link-time), it makes more sense to keep __compiletime_error(), which is also neatly mated with __compiletime_warning(). Signed-off-by: Daniel Santos <daniel.santos@pobox.com> --- include/linux/compiler-gcc4.h | 2 -- include/linux/compiler.h | 3 --- 2 files changed, 0 insertions(+), 5 deletions(-)