Re: [PATCH] Allow build without dirent.d_type, fix "ls (host)" crash

[Christian Franke]

Marco Gerards wrote:

> > ...
> > +static int
> > +is_dir(const char *path, const char *name)
> >     
>
> is_dir (
>
>   

OK. Too many projects, too many policies, sorry :-)


> > +{
> > +  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?
>
>   

??


> > ...
> > @@ -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 ;-)

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.

Christian

2007-11-09  Christian Franke  <address@hidden>

        * 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)".



--- grub2.orig/util/hostfs.c    2007-08-02 19:24:06.000000000 +0200
+++ grub2/util/hostfs.c 2007-11-09 21:41:42.484375000 +0100
@@ -25,6 +25,34 @@
 #include <dirent.h>
 #include <stdio.h>
 
+
+#ifndef DT_DIR
+/* dirent.d_type is a BSD extension, not part of POSIX */
+#include <sys/stat.h>
+#include <string.h>
+
+static int
+is_dir (const char *path, const char *name)
+{
+  int len1 = strlen(path);
+  int len2 = strlen(name);
+
+  char pathname[len1 + 1 + len2 + 1 + 13];
+  strcpy (pathname, path);
+
+  /* Avoid UNC-path "//name" on Cygwin.  */
+  if (len1 > 0 && pathname[len1 - 1] != '/')
+    strcat (pathname, "/");
+
+  strcat (pathname, name);
+
+  struct stat st;
+  if (stat (pathname, &st))
+    return 0;
+  return S_ISDIR (st.st_mode);
+}
+#endif
+
 static grub_err_t
 grub_hostfs_dir (grub_device_t device, const char *path, 
                 int (*hook) (const char *filename, int dir))
@@ -48,7 +76,11 @@
       if (! de)
        break;
 
+#ifdef DT_DIR
       hook (de->d_name, de->d_type == DT_DIR);
+#else
+      hook (de->d_name, is_dir (path, de->d_name));
+#endif
     }
 
   closedir (dir);
@@ -81,7 +113,8 @@
   FILE *f;
 
   f = (FILE *) file->data;
-  int s= fread (buf, 1, len, f);
+  fseek (f, file->offset, SEEK_SET);
+  int s = fread (buf, 1, len, f);
 
   return s;
 }
@@ -101,6 +134,7 @@
 grub_hostfs_label (grub_device_t device __attribute ((unused)),
                   char **label __attribute ((unused)))
 {
+  *label = 0;
   return GRUB_ERR_NONE;
 }