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

[Marco Gerards]

Pavel Roskin <address@hidden> writes:

> On Thu, 2008-07-03 at 20:21 +0200, Marco Gerards wrote:
>> Pavel Roskin <address@hidden> 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.

What I meant is that even *looking* at code might cause problems.
People can claim you have stolen their ideas.  That would essentially
mean the same as copying code.  I just want to avoid such problems at
beforehand.

>> 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.

Right

> 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.

What I suggest is that you pick the names yourself or from a standard,
instead of from Linux code.

--
Marco