2004-01-25 04:49:55

by Bryan Whitehead

[permalink] [raw]
Subject: [PATCH 2.6.2-rc1-mm3] fs/xfs/xfs_log_recover.c

On compile I get this:

fs/xfs/xfs_log_recover.c: In function `xlog_recover_reorder_trans':
fs/xfs/xfs_log_recover.c:1534: warning: `flags' might be used uninitialized in this function

I previously sent this patch and it was wrong.

In the function xlog_recover_reorder_trans the comiler thinks that "flags" might be used uninitialized. This will never happen as the first switch statement takes care of giving flags a value for all CASE statements that will use the flags variable later in the function. To get rid of the warning, flags needs to start off with an initial value. The previous patch merged the 2 switch statements but that would have broken the XFS_LI_BUF case as flags would be overwritten in the XFS_LI_5_3_BUF case.

This patch keeps the same functionality but removes the warning the compiler generates.

--- fs/xfs/xfs_log_recover.c.orig 2004-01-24 20:16:15.726073560 -0800
+++ fs/xfs/xfs_log_recover.c 2004-01-24 20:33:02.720987064 -0800
@@ -1531,7 +1531,7 @@ xlog_recover_reorder_trans(
xlog_recover_item_t *first_item, *itemq, *itemq_next;
xfs_buf_log_format_t *buf_f;
xfs_buf_log_format_v1_t *obuf_f;
- ushort flags;
+ ushort flags = 0;

first_item = itemq = trans->r_itemq;
trans->r_itemq = NULL;

--
Bryan Whitehead
[email protected]


2004-01-25 11:11:26

by Tim Cambrant

[permalink] [raw]
Subject: Re: [PATCH 2.6.2-rc1-mm3] fs/xfs/xfs_log_recover.c

On Sat, Jan 24, 2004 at 08:48:59PM -0800, Bryan Whitehead wrote:
> This patch keeps the same functionality but removes the warning the compiler generates.

I sent you a patch exactly like this a few days ago, but I don't know
if you got it. This way is a lot more simple than the approach you went
for in your last patch, but it really shouldn't matter at all. All it
does is to clear a warning. One tip though, in SubmittingPatches you
can read that the best way to create patches is by making them apply
with the -p1 flag. This is done by including the actual kernel source
directory when making the diff, such as this:

diff -up linux/fs/xfs/xfs_log_recover.c.orig linux/fs/xfs/xfs_log_recover.c

It doesn't matter what you named your kernel directory, since the -p1 flag
ignores that name. Using this command will improve your chances of getting
your patches included.


Tim Cambrant


Attachments:
(No filename) (904.00 B)
(No filename) (189.00 B)
Download all attachments

2004-01-25 21:37:30

by Bryan Whitehead

[permalink] [raw]
Subject: Re: [PATCH 2.6.2-rc1-mm3] fs/xfs/xfs_log_recover.c

Tim Cambrant wrote:
> On Sat, Jan 24, 2004 at 08:48:59PM -0800, Bryan Whitehead wrote:
>
>>This patch keeps the same functionality but removes the warning the compiler generates.
>
>
> I sent you a patch exactly like this a few days ago, but I don't know
> if you got it. This way is a lot more simple than the approach you went
> for in your last patch, but it really shouldn't matter at all. All it
> does is to clear a warning. One tip though, in SubmittingPatches you
> can read that the best way to create patches is by making them apply
> with the -p1 flag. This is done by including the actual kernel source
> directory when making the diff, such as this:

I didn't get it. Sorry.

> diff -up linux/fs/xfs/xfs_log_recover.c.orig linux/fs/xfs/xfs_log_recover.c
>
> It doesn't matter what you named your kernel directory, since the -p1 flag
> ignores that name. Using this command will improve your chances of getting
> your patches included.
>
>
> Tim Cambrant

In Documentation/SubmittingPatches it says this:

To create a patch for a single file, it is often sufficient to do:

SRCTREE= /devel/linux-2.4
MYFILE= drivers/net/mydriver.c

cd $SRCTREE
cp $MYFILE $MYFILE.orig
vi $MYFILE # make your change
diff -up $MYFILE.orig $MYFILE > /tmp/patch

From the example I am supposed to be in my source tree, not just
outside it.

Does the documentation need to be changed? It seems everyone I've sent a
patch to would like a patch that looks like "diff -up
linux/fs/xfs/xfs_log_recover.c.orig linux/fs/xfs/xfs_log_recover.c"
instead of "diff -up fs/xfs/xfs_log_recover.c.orig
fs/xfs/xfs_log_recover.c".

If this is the case I wouldn't mind updating the docs and submitting a
patch. ;)

--
Bryan Whitehead
Email:[email protected]
WorkE:[email protected]

2004-01-26 23:46:34

by Nathan Scott

[permalink] [raw]
Subject: Re: [PATCH 2.6.2-rc1-mm3] fs/xfs/xfs_log_recover.c

On Sat, Jan 24, 2004 at 08:48:59PM -0800, Bryan Whitehead wrote:
> On compile I get this:
>
> fs/xfs/xfs_log_recover.c: In function `xlog_recover_reorder_trans':
> fs/xfs/xfs_log_recover.c:1534: warning: `flags' might be used uninitialized in this function
>
> I previously sent this patch and it was wrong.

What compiler version are you using? Is this a recent gcc or an
older version - if the former, is gcc really getting dumber? if
the latter, I'm wondering why I haven't come across this anytime
in the last few years of compiling xfs. Or is this some non-gcc
compiler out of left field?

thanks.

--
Nathan

2004-01-27 00:10:18

by Bryan Whitehead

[permalink] [raw]
Subject: Re: [PATCH 2.6.2-rc1-mm3] fs/xfs/xfs_log_recover.c

Nathan Scott wrote:
> On Sat, Jan 24, 2004 at 08:48:59PM -0800, Bryan Whitehead wrote:
>
>>On compile I get this:
>>
>>fs/xfs/xfs_log_recover.c: In function `xlog_recover_reorder_trans':
>>fs/xfs/xfs_log_recover.c:1534: warning: `flags' might be used uninitialized in this function
>>
>>I previously sent this patch and it was wrong.
>
>
> What compiler version are you using? Is this a recent gcc or an
> older version - if the former, is gcc really getting dumber? if
> the latter, I'm wondering why I haven't come across this anytime
> in the last few years of compiling xfs. Or is this some non-gcc
> compiler out of left field?
>
> thanks.
>

This is gcc 3.3.2 on gentoo linux.

gcc may be getting dumber, or just more precautious?

--
Bryan Whitehead
Email:[email protected]
WorkE:[email protected]

2004-01-27 00:20:36

by Nathan Scott

[permalink] [raw]
Subject: Re: [PATCH 2.6.2-rc1-mm3] fs/xfs/xfs_log_recover.c

On Tue, Jan 27, 2004 at 10:41:59AM +1100, Nathan Scott wrote:
> On Sat, Jan 24, 2004 at 08:48:59PM -0800, Bryan Whitehead wrote:
> > On compile I get this:
> >
> > fs/xfs/xfs_log_recover.c: In function `xlog_recover_reorder_trans':
> > fs/xfs/xfs_log_recover.c:1534: warning: `flags' might be used uninitialized in this function
> >
> > I previously sent this patch and it was wrong.
>
> What compiler version are you using? Is this a recent gcc or an

Sorry, I'm still half a sleep -- I see this too with the recent
fix in this routine, looks like many current gcc versions report
it. I guess initialising flags to zero will be the simplest way
to workaround it.

thanks.

--
Nathan