Christian Franke <[EMAIL PROTECTED]> writes:
> Marco Gerards wrote:
>
>>> ...
>>> +static int
>>> +is_dir(const char *path, const char *name)
>>>
>>
>> is_dir (
>>
>>
>
> OK. Too many projects, too many policies, sorry :-)
np, don't worry :-)
Hopefully you are not annoyed by my comments. A consistent style
throughout GRUB is important to me.
>>> +{
>>> + int len1 = strlen(path), len2 = strlen(name);
>>>
>>
>> Please split this up. Or even better use separate declarations and
>> code.
>>
>>
>
> Yes. No. See comment below.
>
>>> + char pathname[len1+1+len2+1+13];
>>>
>>
>> Please add spaces around binary operators.
>>
>>
>>> + struct stat st;
>>> + strcpy (pathname, path);
>>>
>>
>> Please add more blank lines between your code to make it easier to
>> read. :-)
>>
>>
>
> OK.
>
>
>>> ...
>>> + strcat (pathname, "/");
>>> + strcat (pathname, name);
>>> + if (stat (pathname, &st))
>>> + return 0;
>>> + return S_ISDIR(st.st_mode);
>>> +}
>>> +#endif
>>>
>>
>> Why can't you just use S_ISDIR?
>>
>>
>
> ??
Nevermind, I wasn't really awake ;)
>>> ...
>>> @@ -81,6 +108,7 @@ grub_hostfs_read (grub_file_t file, char
>>> FILE *f;
>>> f = (FILE *) file->data;
>>> + fseek (f, file->offset, SEEK_SET);
>>> int s= fread (buf, 1, len, f);
>>>
>>
>> "int s = "
>>
>>
>
> Code janitor work outside the scope of this patch ... done ;-)
LOL! Sorry, I was a bit enthousiastic ;-)
Thanks! :)
> BTW: This line uses "declaration statement" syntax, therefore this is
> apparently accepted in GRUB2 codebase :-)
> I definitely prefer this (first use = declaration = initialization)
> style, which is state of the art in most (all?) modern languages.
> This C++ (1986) feature and early GCC (1.*) extension was finally
> included into C99, so it is portable now.
> is_dir() rewritten accordingly.
It is. The problem is that you had two function calls on one line. I
want to avoid that.
> Christian
>
> 2007-11-09 Christian Franke <[EMAIL PROTECTED]>
>
> * util/hostfs.c (is_dir): New function.
> (grub_hostfs_dir): Handle missing dirent.d_type case.
> (grub_hostfs_read): Add missing fseek().
> (grub_hostfs_label): Clear label pointer. This fixes a crash
> of grub-emu on "ls (host)".
Looks fine to me.
btw, when do you use (host)? It's intended as a debugging tool.
--
Marco
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel
