While porting to 2.6 I am just using the 2.6 build system and putting off the autoconfig stuff till later. As a result of this I am seeing a ton of additional warning messages due to the fact that the ocfs2 build system is including "-Wno-format". Is there a good reason for adding the "-Wno-format" option to the build? The kind of mistakes that are being ignored are ==> LOG_TRACE_ARGS("found some data to free (%u.%u)\n", HI(cur_extent->this_ext), LO(cur_extent->this_ext)); alloc.c: In function `ocfs_kill_this_tree': alloc.c:1620: warning: unsigned int format, long unsigned int arg (arg 2) alloc.c:1620: warning: unsigned int format, long unsigned int arg (arg 3) alloc.c:1638: warning: unsigned int format, long unsigned int arg (arg 2) alloc.c:1638: warning: unsigned int format, long unsigned int arg (arg 3) and ==> LOG_TRACE_ARGS("Popping this header (%u.%u)\n", HI(AllocExtent->this_ext), LO(AllocExtent->this_ext), AllocExtent->next_free_ext); alloc.c:2079: warning: unsigned int format, long unsigned int arg (arg 2) alloc.c:2079: warning: unsigned int format, long unsigned int arg (arg 3) alloc.c:2079: warning: too many arguments for format alloc.c:2099: warning: unsigned int format, long unsigned int arg (arg 2) alloc.c:2099: warning: unsigned int format, long unsigned int arg (arg 3) If it is ok, I would like to make the build more strict, and treat each of the warnings as bugs. --rusty
On Thu, Feb 12, 2004 at 10:39:27AM -0800, Rusty Lynch wrote:> While porting to 2.6 I am just using the 2.6 build system and putting off > the autoconfig stuff till later. As a result of this I am seeing a ton of > additional warning messages due to the fact that the ocfs2 build system is > including "-Wno-format". > > Is there a good reason for adding the "-Wno-format" option to the build? > > The kind of mistakes that are being ignored are ==> > > LOG_TRACE_ARGS("found some data to free (%u.%u)\n", > HI(cur_extent->this_ext), LO(cur_extent->this_ext)); > alloc.c: In function `ocfs_kill_this_tree': > alloc.c:1620: warning: unsigned int format, long unsigned int arg (arg 2) > alloc.c:1620: warning: unsigned int format, long unsigned int arg (arg 3) > alloc.c:1638: warning: unsigned int format, long unsigned int arg (arg 2) > alloc.c:1638: warning: unsigned int format, long unsigned int arg (arg 3) > > > and ==> > LOG_TRACE_ARGS("Popping this header (%u.%u)\n", > HI(AllocExtent->this_ext), > LO(AllocExtent->this_ext), > AllocExtent->next_free_ext); > alloc.c:2079: warning: unsigned int format, long unsigned int arg (arg 2) > alloc.c:2079: warning: unsigned int format, long unsigned int arg (arg 3) > alloc.c:2079: warning: too many arguments for format > alloc.c:2099: warning: unsigned int format, long unsigned int arg (arg 2) > alloc.c:2099: warning: unsigned int format, long unsigned int arg (arg 3) > > > If it is ok, I would like to make the build more strict, and treat each of > the warnings as bugs. > > --rustyand here is a quick patch to remove the flag Index: src/Makefile ==================================================================--- src/Makefile (revision 31) +++ src/Makefile (working copy) @@ -2,7 +2,7 @@ include $(TOPDIR)/Preamble.make -WARNINGS = -Wall -Wstrict-prototypes -Wno-format +WARNINGS = -Wall -Wstrict-prototypes ifneq ($(OCFS_PROCESSOR),x86_64) WARNINGS += -Wmissing-prototypes -Wmissing-declarations
On Thu, Feb 12, 2004 at 10:39:27AM -0800, Rusty Lynch wrote:> While porting to 2.6 I am just using the 2.6 build system and putting off > the autoconfig stuff till later. As a result of this I am seeing a ton of > additional warning messages due to the fact that the ocfs2 build system is > including "-Wno-format". > > Is there a good reason for adding the "-Wno-format" option to the build?No good reason. Fixing the LOG messages for proper formats was deemed less important than fixing real functionality bugs, so -Wno-format is used to cut down on the warning noise.> The kind of mistakes that are being ignored are ==> > > LOG_TRACE_ARGS("found some data to free (%u.%u)\n", > HI(cur_extent->this_ext), LO(cur_extent->this_ext)); > alloc.c: In function `ocfs_kill_this_tree': > alloc.c:1620: warning: unsigned int format, long unsigned int arg (arg 2) > alloc.c:1620: warning: unsigned int format, long unsigned int arg (arg 3) > alloc.c:1638: warning: unsigned int format, long unsigned int arg (arg 2) > alloc.c:1638: warning: unsigned int format, long unsigned int arg (arg 3) > > > and ==> > LOG_TRACE_ARGS("Popping this header (%u.%u)\n", > HI(AllocExtent->this_ext), > LO(AllocExtent->this_ext), > AllocExtent->next_free_ext); > alloc.c:2079: warning: unsigned int format, long unsigned int arg (arg 2) > alloc.c:2079: warning: unsigned int format, long unsigned int arg (arg 3) > alloc.c:2079: warning: too many arguments for format > alloc.c:2099: warning: unsigned int format, long unsigned int arg (arg 2) > alloc.c:2099: warning: unsigned int format, long unsigned int arg (arg 3) > > > If it is ok, I would like to make the build more strict, and treat each of > the warnings as bugs.Go for it, the cleanup is welcome. While you're at it, getting rid of the HI()/LO() nonsense would be good too. -Manish
Code in OpenGFS does something similar, but with a smaller macro label: FMT_64 It's just a little more compact than Rusty's suggestion. -- Ben --> -----Original Message----- > From: ocfs2-devel-bounces@oss.oracle.com > [mailto:ocfs2-devel-bounces@oss.oracle.com] On Behalf Of Rusty Lynch > Sent: Friday, February 13, 2004 10:22 PM > To: Mark Fasheh > Cc: ocfs2-devel@oss.oracle.com > Subject: Re: [Ocfs2-devel] Request to remove -Wfno-format > > > On Fri, Feb 13, 2004 at 07:18:41PM -0800, Mark Fasheh wrote: > > On Fri, Feb 13, 2004 at 07:09:40PM -0800, Rusty Lynch wrote: > > > I have something here that instead of doing crazy cast, > with define a U64_MODIFIER > > > that will expand appropriatly. So... it's always correct > and much easier on the eyes. > > > > > > printk's would look something like: > > > > > > printk(KERN_DEBUG "size_t is %Zu, and a u64 is " > U64_MODIFIER "\n", asize, au64value); > > > > > > Let me do one more verification on the patch and I will > send it out. > > > > > > --rusty > > > > Have you seen how often we print out __u64's? I predict > that chopping up our > > format strings with that will be much uglier than a cast macro ;) > > --Mark > > > > Take a look at the following and see if it's too much. > > --rusty > >