From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with archive (Exim 4.43) id 1NNY8P-0002Ig-Fw for mharc-grub-devel@gnu.org; Wed, 23 Dec 2009 15:48:41 -0500 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1NNY8M-0002De-Nj for grub-devel@gnu.org; Wed, 23 Dec 2009 15:48:38 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1NNY8H-00025x-Mz for grub-devel@gnu.org; Wed, 23 Dec 2009 15:48:38 -0500 Received: from [199.232.76.173] (port=44126 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1NNY8H-00025j-DY for grub-devel@gnu.org; Wed, 23 Dec 2009 15:48:33 -0500 Received: from iona.labri.fr ([147.210.8.143]:36909) by monty-python.gnu.org with esmtps (TLS-1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1NNY8H-0004B8-5k for grub-devel@gnu.org; Wed, 23 Dec 2009 15:48:33 -0500 Received: from localhost (localhost.localdomain [127.0.0.1]) by iona.labri.fr (Postfix) with ESMTP id 7681A36B83 for ; Wed, 23 Dec 2009 21:48:30 +0100 (CET) X-Virus-Scanned: amavisd-new at labri.fr Received: from iona.labri.fr ([127.0.0.1]) by localhost (iona.labri.fr [127.0.0.1]) (amavisd-new, port 10027) with LMTP id IHIbG23SGePB for ; Wed, 23 Dec 2009 21:48:30 +0100 (CET) Received: from [192.168.1.50] (c2433-1-88-160-112-182.fbx.proxad.net [88.160.112.182]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by iona.labri.fr (Postfix) with ESMTP id 251B136B7A for ; Wed, 23 Dec 2009 21:48:30 +0100 (CET) Message-ID: <4B32821F.6050607@labri.fr> Date: Wed, 23 Dec 2009 21:48:31 +0100 From: =?ISO-8859-1?Q?Gr=E9goire_Sutre?= User-Agent: Mozilla-Thunderbird 2.0.0.22 (X11/20090707) MIME-Version: 1.0 To: The development of GNU GRUB Content-Type: multipart/mixed; boundary="------------010809010100060808020009" X-detected-operating-system: by monty-python.gnu.org: GNU/Linux 2.6 (newer, 3) Subject: Non-static variables and nested function pointers [bug #28392] X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: The development of GNU GRUB List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 23 Dec 2009 20:48:39 -0000 This is a multi-part message in MIME format. --------------010809010100060808020009 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: quoted-printable Hi, I am trying to add NetBSD specific code to util/hostdisk.c in order to=20 make grub-probe work. This part is almost finished. However, I had a=20 hard time dealing with segfaults in callbacks (hook function pointers)=20 in a number of places of the vanilla code. Actually, I get segfaults in=20 grub-probe with the vanilla trunk code (see bug report #28392). This is=20 on NetBSD 5.0 i386. In the end, these segfaults were fixed by making sure that all variables=20 accessed by pointers to nested functions are declared static. I attach=20 a patch that fixes these segfaults on my NetBSD box (this patch is also=20 included in the bug report). However, I am not a C expert, and I must be missing something as the=20 code (obviously) works well on other systems. Thanks for your help, Gr=E9goire --------------010809010100060808020009 Content-Type: text/x-patch; name="patch-nested-functions.diff" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="patch-nested-functions.diff" 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); --------------010809010100060808020009--