grub-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: JFS support (PATCH) and filesystem improvements


From: Tomas Ebenlendr
Subject: Re: JFS support (PATCH) and filesystem improvements
Date: Mon, 30 Aug 2004 01:00:44 +0200
User-agent: Mutt/1.5.6i

Only some comments of very small importance as proof that I have
read the patch.

--- slashes: ---
sub-function find_file():

> +      if (*name == '/')
> +     name++;

what about 'while' ? I mean if we remove first one slash, why not the
second (as in linux, a sequence of slashes acts as one slash).

> +  
> +      /* Remove trailing "/".  */
> +      if (name[grub_strlen (name) - 1] == '/')
> +     name[grub_strlen (name) - 1] = '\0';

This is not need if we skip _all_ slashes ...

> +       /* Extract the actual part from the pathname.  */
> +       next = grub_strchr (name, '/');
> +       if (next)
> +         {
> +           next[0] = '\0';
> +           next++;
> +         }

... here. I propose: while (*next == '/') next++; instead of simply
incrementing.
--- :slashes end ---

> +           /* The symlink is an absolute path, go back to the root inode.  */
> +           if (symlink[0] == '/')
> +             {
> +               free_node (oldnode);
> +               oldnode = rootnode;
> +             }
And for relative path ... :
Please write somewhere that you expect correct behavior for '..'
entry. (e.g., in fshelp.h). When someone will implement weird fs,
he must know what this function expects.

> +       
> +       /* Fond the node!  */
Typo in comment :-).

> +      /* First block.  */
> +      if (i == pos / blocksize)
> +     {
> +       skipfirst = blockoff;
> +       blockend -= skipfirst;
> +     }
blockend ... not a good name: blockbytes? (hmm, also not very good.)

-- 
                                 Tomas 'ebi' Ebenlendr
                                 http://get.to/ebik
                                 PF 2004.66118890407





reply via email to

[Prev in Thread] Current Thread [Next in Thread]