Re: [PATCH] Fix warning in fs/xfs.c

[Pavel Roskin <proski@gnu.org>]

On Thu, 2008-07-03 at 20:21 +0200, Marco Gerards wrote:
> Pavel Roskin <proski@gnu.org> writes:
> 
> > ChangeLog:
> > 	* fs/xfs.c (struct grub_xfs_dir_header): Use names similar to
> > 	those in Linux XFS code.  Provide a way to access 64-bit parent
> > 	inode.
> > 	(grub_xfs_iterate_dir): Use the new names.  Avoid reading past
> > 	the end of struct grub_xfs_dir_header.
> 
> *please* do not look at Linux code or whatever *and* contribute to
>  GRUB.  It might cause copyright troubles I will have to deal with :-/

I just tried to make names similar without copying any code.  But it's a
useful reminder.

> I do not see the advantage of this patch.  Can you please explain why
> we need these name changes?

We were casting a pointer to a 32-bit integer to a pointer to a 64-bit
integer, which is bad, and gcc was emitting a warning about it.

Worse yet, the 64-bit value was "sticking" beyond the end the structure
we were using to describe the header.

i4 and i8 are generally used by Linux XFS code to describe 32-bit and
64-bit values if either can be used.  The "smallino" field was highly
misleading because it had to be negated.  It's the number of "big" (i8
or 64-bit) entries.  If it's 0, then the entries are "small".

So it was natural to call it "i8count".  And once it was "i8count", it
was natural to call the first value "count".

If you prefer another naming convention, let's rename the entries
according to it.  I was thinking having 2 32-bit integers "parent_hi"
and "parent_lo" or something like that.  Anyway, let's not use
"smallino" - "bigentries" would be better.

-- 
Regards,
Pavel Roskin