Non-static variables and nested function pointers [bug #28392]

["Grégoire Sutre" <gregoire.sutre@labri.fr>]

[-- Attachment #1: Type: text/plain, Size: 809 bytes --]

Hi,

I am trying to add NetBSD specific code to util/hostdisk.c in order to 
make grub-probe work.  This part is almost finished.  However, I had a 
hard time dealing with segfaults in callbacks (hook function pointers) 
in a number of places of the vanilla code.  Actually, I get segfaults in 
grub-probe with the vanilla trunk code (see bug report #28392).  This is 
on NetBSD 5.0 i386.

In the end, these segfaults were fixed by making sure that all variables 
accessed by pointers to nested functions are declared static.  I attach 
a patch that fixes these segfaults on my NetBSD box (this patch is also 
included in the bug report).

However, I am not a C expert, and I must be missing something as the 
code (obviously) works well on other systems.

Thanks for your help,

Grégoire

[-- Attachment #2: patch-nested-functions.diff --]
[-- Type: text/x-patch, Size: 7659 bytes --]

diff -Naur grub2_2009-12-22/fs/ext2.c grub2/fs/ext2.c
--- grub2_2009-12-22/fs/ext2.c	2009-12-22 17:53:27.000000000 +0100
+++ grub2/fs/ext2.c	2009-12-23 19:05:36.000000000 +0100
@@ -789,9 +789,12 @@
 	       int (*hook) (const char *filename,
 			    const struct grub_dirhook_info *info))
 {
-  struct grub_ext2_data *data = 0;
+  static struct grub_ext2_data *data = 0;
   struct grub_fshelp_node *fdiro = 0;
 
+  static int (*myhook) (const char *filename,
+                        const struct grub_dirhook_info *info);
+
   auto int NESTED_FUNC_ATTR iterate (const char *filename,
 				     enum grub_fshelp_filetype filetype,
 				     grub_fshelp_node_t node);
@@ -817,9 +820,11 @@
 
       info.dir = ((filetype & GRUB_FSHELP_TYPE_MASK) == GRUB_FSHELP_DIR);
       grub_free (node);
-      return hook (filename, &info);
+      return myhook (filename, &info);
     }
 
+  myhook = hook;
+
   grub_dl_ref (my_mod);
 
   data = grub_ext2_mount (device->disk);
diff -Naur grub2_2009-12-22/fs/fat.c grub2/fs/fat.c
--- grub2_2009-12-22/fs/fat.c	2009-12-22 17:53:27.000000000 +0100
+++ grub2/fs/fat.c	2009-12-23 17:35:00.000000000 +0100
@@ -606,9 +606,13 @@
 		   int (*hook) (const char *filename,
 				const struct grub_dirhook_info *info))
 {
-  char *dirname, *dirp;
-  int call_hook;
-  int found = 0;
+  static char *dirname;
+  char *dirp;
+  static int call_hook;
+  static int found = 0;
+  static struct grub_fat_data *mydata;
+  static int (*myhook) (const char *filename,
+                        const struct grub_dirhook_info *info);
 
   auto int iter_hook (const char *filename, struct grub_fat_dir_entry *dir);
   int iter_hook (const char *filename, struct grub_fat_dir_entry *dir)
@@ -622,25 +626,28 @@
     if (dir->attr & GRUB_FAT_ATTR_VOLUME_ID)
       return 0;
     if (*dirname == '\0' && call_hook)
-      return hook (filename, &info);
+      return myhook (filename, &info);
 
     if (grub_strcasecmp (dirname, filename) == 0)
       {
 	found = 1;
-	data->attr = dir->attr;
-	data->file_size = grub_le_to_cpu32 (dir->file_size);
-	data->file_cluster = ((grub_le_to_cpu16 (dir->first_cluster_high) << 16)
+	mydata->attr = dir->attr;
+	mydata->file_size = grub_le_to_cpu32 (dir->file_size);
+	mydata->file_cluster = ((grub_le_to_cpu16 (dir->first_cluster_high) << 16)
 			      | grub_le_to_cpu16 (dir->first_cluster_low));
-	data->cur_cluster_num = ~0U;
+	mydata->cur_cluster_num = ~0U;
 
 	if (call_hook)
-	  hook (filename, &info);
+	  myhook (filename, &info);
 
 	return 1;
       }
     return 0;
   }
 
+  mydata = data;
+  myhook = hook;
+
   if (! (data->attr & GRUB_FAT_ATTR_DIRECTORY))
     {
       grub_error (GRUB_ERR_BAD_FILE_TYPE, "not a directory");
diff -Naur grub2_2009-12-22/kern/device.c grub2/kern/device.c
--- grub2_2009-12-22/kern/device.c	2009-12-22 17:53:30.000000000 +0100
+++ grub2/kern/device.c	2009-12-23 18:43:21.000000000 +0100
@@ -83,17 +83,45 @@
   auto int iterate_partition (grub_disk_t disk,
 			      const grub_partition_t partition);
 
-  struct part_ent
+  static struct part_ent
   {
     struct part_ent *next;
     char name[0];
   } *ents;
 
+  static int (*myhook) (const char *name);
+
+  int iterate_partition (grub_disk_t disk, const grub_partition_t partition)
+    {
+      char *partition_name;
+      struct part_ent *p;
+
+      partition_name = grub_partition_get_name (partition);
+      if (! partition_name)
+	return 1;
+
+      p = grub_malloc (sizeof (p->next) + grub_strlen (disk->name) + 1 +
+		       grub_strlen (partition_name) + 1);
+      if (!p)
+	{
+	  grub_free (partition_name);
+	  return 1;
+	}
+
+      grub_sprintf (p->name, "%s,%s", disk->name, partition_name);
+      grub_free (partition_name);
+
+      p->next = ents;
+      ents = p;
+
+      return 0;
+    }
+
   int iterate_disk (const char *disk_name)
     {
       grub_device_t dev;
 
-      if (hook (disk_name))
+      if (myhook (disk_name))
 	return 1;
 
       dev = grub_device_open (disk_name);
@@ -117,7 +145,7 @@
 	      struct part_ent *next = p->next;
 
 	      if (!ret)
-		ret = hook (p->name);
+		ret = myhook (p->name);
 	      grub_free (p);
 	      p = next;
 	    }
@@ -129,31 +157,7 @@
       return 0;
     }
 
-  int iterate_partition (grub_disk_t disk, const grub_partition_t partition)
-    {
-      char *partition_name;
-      struct part_ent *p;
-
-      partition_name = grub_partition_get_name (partition);
-      if (! partition_name)
-	return 1;
-
-      p = grub_malloc (sizeof (p->next) + grub_strlen (disk->name) + 1 +
-		       grub_strlen (partition_name) + 1);
-      if (!p)
-	{
-	  grub_free (partition_name);
-	  return 1;
-	}
-
-      grub_sprintf (p->name, "%s,%s", disk->name, partition_name);
-      grub_free (partition_name);
-
-      p->next = ents;
-      ents = p;
-
-      return 0;
-    }
+  myhook = hook;
 
   /* Only disk devices are supported at the moment.  */
   return grub_disk_dev_iterate (iterate_disk);
diff -Naur grub2_2009-12-22/kern/partition.c grub2/kern/partition.c
--- grub2_2009-12-22/kern/partition.c	2009-12-22 17:53:30.000000000 +0100
+++ grub2/kern/partition.c	2009-12-23 17:52:43.000000000 +0100
@@ -57,13 +57,15 @@
 grub_partition_t
 grub_partition_probe (struct grub_disk *disk, const char *str)
 {
-  grub_partition_t part = 0;
+  static grub_partition_t part = 0;
+  static struct grub_disk *mydisk;
+  static const char *mystr;
 
   auto int part_map_probe (const grub_partition_map_t partmap);
 
   int part_map_probe (const grub_partition_map_t partmap)
     {
-      part = partmap->probe (disk, str);
+      part = partmap->probe (mydisk, mystr);
       if (part)
 	return 1;
 
@@ -77,6 +79,9 @@
       return 1;
     }
 
+  mydisk = disk;
+  mystr = str;
+
   /* Use the first partition map type found.  */
   grub_partition_map_iterate (part_map_probe);
 
@@ -88,8 +93,9 @@
 			int (*hook) (grub_disk_t disk,
 				     const grub_partition_t partition))
 {
-  grub_partition_map_t partmap = 0;
+  static grub_partition_map_t partmap = 0;
   int ret = 0;
+  static struct grub_disk *mydisk;
 
   auto int part_map_iterate (const grub_partition_map_t p);
   auto int part_map_iterate_hook (grub_disk_t d,
@@ -104,7 +110,7 @@
   int part_map_iterate (const grub_partition_map_t p)
     {
       grub_dprintf ("partition", "Detecting %s...\n", p->name);
-      p->iterate (disk, part_map_iterate_hook);
+      p->iterate (mydisk, part_map_iterate_hook);
 
       if (grub_errno != GRUB_ERR_NONE)
 	{
@@ -119,6 +125,8 @@
       return 1;
     }
 
+  mydisk = disk;
+
   grub_partition_map_iterate (part_map_iterate);
   if (partmap)
     ret = partmap->iterate (disk, hook);
diff -Naur grub2_2009-12-22/partmap/msdos.c grub2/partmap/msdos.c
--- grub2_2009-12-22/partmap/msdos.c	2009-12-22 17:53:31.000000000 +0100
+++ grub2/partmap/msdos.c	2009-12-23 14:19:35.000000000 +0100
@@ -251,8 +251,8 @@
 static grub_partition_t
 pc_partition_map_probe (grub_disk_t disk, const char *str)
 {
-  grub_partition_t p;
-  struct grub_msdos_partition *pcdata;
+  static grub_partition_t p;
+  static struct grub_msdos_partition *pcdata;
 
   auto int find_func (grub_disk_t d, const grub_partition_t partition);
 
diff -Naur grub2_2009-12-22/partmap/sun.c grub2/partmap/sun.c
--- grub2_2009-12-22/partmap/sun.c	2009-12-22 17:53:31.000000000 +0100
+++ grub2/partmap/sun.c	2009-12-23 17:44:40.000000000 +0100
@@ -141,8 +141,8 @@
 static grub_partition_t
 sun_partition_map_probe (grub_disk_t disk, const char *str)
 {
-  grub_partition_t p = 0;
-  int partnum = 0;
+  static grub_partition_t p = 0;
+  static int partnum = 0;
   char *s = (char *) str;
 
   auto int find_func (grub_disk_t d, const grub_partition_t partition);