* [PATCH] switch from sprintf to asprintf and snprintf
@ 2009-12-29 9:30 Vladimir 'φ-coder/phcoder' Serbinenko
2010-01-01 9:32 ` Colin Watson
2010-01-01 11:51 ` Robert Millan
0 siblings, 2 replies; 11+ messages in thread
From: Vladimir 'φ-coder/phcoder' Serbinenko @ 2009-12-29 9:30 UTC (permalink / raw)
To: The development of GRUB 2
[-- Attachment #1.1: Type: text/plain, Size: 200 bytes --]
sprintf is potentially dangerous especially with gettext, when messages
may be larger than coder would expect. I attach the patch to fix it
--
Regards
Vladimir 'φ-coder/phcoder' Serbinenko
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: asprintf.diff --]
[-- Type: text/x-diff; name="asprintf.diff", Size: 54677 bytes --]
=== modified file 'commands/ls.c'
--- commands/ls.c 2009-12-25 23:50:59 +0000
+++ commands/ls.c 2009-12-29 09:04:06 +0000
@@ -86,7 +86,7 @@
int print_files_long (const char *filename,
const struct grub_dirhook_info *info)
{
- char pathname[grub_strlen (dirname) + grub_strlen (filename) + 1];
+ char *pathname;
if ((! all) && (filename[0] == '.'))
return 0;
@@ -96,9 +96,12 @@
grub_file_t file;
if (dirname[grub_strlen (dirname) - 1] == '/')
- grub_sprintf (pathname, "%s%s", dirname, filename);
+ pathname = grub_asprintf ("%s%s", dirname, filename);
else
- grub_sprintf (pathname, "%s/%s", dirname, filename);
+ pathname = grub_asprintf ("%s/%s", dirname, filename);
+
+ if (!pathname)
+ return 1;
/* XXX: For ext2fs symlinks are detected as files while they
should be reported as directories. */
@@ -130,8 +133,9 @@
grub_uint32_t whole, fraction;
whole = grub_divmod64 (fsize, 100, &fraction);
- grub_sprintf (buf, "%u.%02u%c", whole, fraction,
- grub_human_sizes[units]);
+ grub_snprintf (buf, sizeof (buf),
+ "%u.%02u%c", whole, fraction,
+ grub_human_sizes[units]);
grub_printf ("%-12s", buf);
}
else
=== modified file 'commands/memrw.c'
--- commands/memrw.c 2009-12-25 23:50:59 +0000
+++ commands/memrw.c 2009-12-29 09:04:06 +0000
@@ -61,7 +61,7 @@
if (cmd->state[0].set)
{
- grub_sprintf (buf, "%x", value);
+ grub_snprintf (buf, sizeof (buf), "%x", value);
grub_env_set (cmd->state[0].arg, buf);
}
else
=== modified file 'commands/parttool.c'
--- commands/parttool.c 2009-12-25 23:50:59 +0000
+++ commands/parttool.c 2009-12-29 09:04:06 +0000
@@ -182,12 +182,11 @@
{
char *filename;
- filename = grub_malloc (grub_strlen (prefix) + sizeof ("/parttool.lst"));
+ filename = grub_asprintf ("%s/parttool.lst", prefix);
if (filename)
{
grub_file_t file;
- grub_sprintf (filename, "%s/parttool.lst", prefix);
file = grub_file_open (filename);
if (file)
{
=== modified file 'commands/search.c'
--- commands/search.c 2009-12-25 23:50:59 +0000
+++ commands/search.c 2009-12-29 09:04:06 +0000
@@ -33,7 +33,6 @@
FUNC_NAME (const char *key, const char *var, int no_floppy)
{
int count = 0;
- char *buf = NULL;
grub_fs_autoload_hook_t saved_autoload;
auto int iterate_device (const char *name);
@@ -48,24 +47,20 @@
#ifdef DO_SEARCH_FILE
{
- grub_size_t len;
- char *p;
+ char *buf;
grub_file_t file;
- len = grub_strlen (name) + 2 + grub_strlen (key) + 1;
- p = grub_realloc (buf, len);
- if (! p)
+ buf = grub_asprintf ("(%s)%s", name, key);
+ if (! buf)
return 1;
- buf = p;
- grub_sprintf (buf, "(%s)%s", name, key);
-
file = grub_file_open (buf);
if (file)
{
found = 1;
grub_file_close (file);
}
+ grub_free (buf);
}
#else
{
@@ -135,8 +130,6 @@
else
grub_device_iterate (iterate_device);
- grub_free (buf);
-
if (grub_errno == GRUB_ERR_NONE && count == 0)
grub_error (GRUB_ERR_FILE_NOT_FOUND, "no such device: %s", key);
}
=== modified file 'commands/xnu_uuid.c'
--- commands/xnu_uuid.c 2009-12-25 23:50:59 +0000
+++ commands/xnu_uuid.c 2009-12-29 09:04:06 +0000
@@ -349,18 +349,18 @@
grub_memcpy (hashme.prefix, hash_prefix, sizeof (hashme.prefix));
md5 ((char *) &hashme, sizeof (hashme), (char *) xnu_uuid);
- grub_sprintf (uuid_string,
- "%02x%02x%02x%02x-%02x%02x-%02x%02x-%02x%02x-%02x%02x%02x%02x%02x%02x",
- (unsigned int) xnu_uuid[0], (unsigned int) xnu_uuid[1],
- (unsigned int) xnu_uuid[2], (unsigned int) xnu_uuid[3],
- (unsigned int) xnu_uuid[4], (unsigned int) xnu_uuid[5],
- (unsigned int) ((xnu_uuid[6] & 0xf) | 0x30),
- (unsigned int) xnu_uuid[7],
- (unsigned int) ((xnu_uuid[8] & 0x3f) | 0x80),
- (unsigned int) xnu_uuid[9],
- (unsigned int) xnu_uuid[10], (unsigned int) xnu_uuid[11],
- (unsigned int) xnu_uuid[12], (unsigned int) xnu_uuid[13],
- (unsigned int) xnu_uuid[14], (unsigned int) xnu_uuid[15]);
+ grub_snprintf (uuid_string, sizeof (uuid_string),
+ "%02x%02x%02x%02x-%02x%02x-%02x%02x-%02x%02x-%02x%02x%02x%02x%02x%02x",
+ (unsigned int) xnu_uuid[0], (unsigned int) xnu_uuid[1],
+ (unsigned int) xnu_uuid[2], (unsigned int) xnu_uuid[3],
+ (unsigned int) xnu_uuid[4], (unsigned int) xnu_uuid[5],
+ (unsigned int) ((xnu_uuid[6] & 0xf) | 0x30),
+ (unsigned int) xnu_uuid[7],
+ (unsigned int) ((xnu_uuid[8] & 0x3f) | 0x80),
+ (unsigned int) xnu_uuid[9],
+ (unsigned int) xnu_uuid[10], (unsigned int) xnu_uuid[11],
+ (unsigned int) xnu_uuid[12], (unsigned int) xnu_uuid[13],
+ (unsigned int) xnu_uuid[14], (unsigned int) xnu_uuid[15]);
for (ptr = uuid_string; *ptr; ptr++)
*ptr = grub_toupper (*ptr);
if (argc == 1)
=== modified file 'disk/ata.c'
--- disk/ata.c 2009-12-25 00:04:51 +0000
+++ disk/ata.c 2009-12-29 09:04:06 +0000
@@ -648,12 +648,14 @@
for (dev = grub_ata_devices; dev; dev = dev->next)
{
- char devname[5];
- grub_sprintf (devname, "ata%d", dev->port * 2 + dev->device);
+ char devname[10];
if (dev->atapi)
continue;
+ grub_snprintf (devname, sizeof (devname),
+ "ata%d", dev->port * 2 + dev->device);
+
if (hook (devname))
return 1;
}
@@ -668,8 +670,9 @@
for (dev = grub_ata_devices; dev; dev = dev->next)
{
- char devname[5];
- grub_sprintf (devname, "ata%d", dev->port * 2 + dev->device);
+ char devname[10];
+ grub_snprintf (devname, sizeof (devname),
+ "ata%d", dev->port * 2 + dev->device);
if (grub_strcmp (name, devname) == 0)
break;
}
@@ -735,8 +738,9 @@
for (dev = grub_ata_devices; dev; dev = dev->next)
{
- char devname[7];
- grub_sprintf (devname, "ata%d", dev->port * 2 + dev->device);
+ char devname[10];
+ grub_snprintf (devname, sizeof (devname),
+ "ata%d", dev->port * 2 + dev->device);
if (! dev->atapi)
continue;
@@ -808,8 +812,9 @@
for (dev = grub_ata_devices; dev; dev = dev->next)
{
- char devname[7];
- grub_sprintf (devname, "ata%d", dev->port * 2 + dev->device);
+ char devname[10];
+ grub_snprintf (devname, sizeof (devname),
+ "ata%d", dev->port * 2 + dev->device);
if (!grub_strcmp (devname, name))
{
=== modified file 'disk/efi/efidisk.c'
--- disk/efi/efidisk.c 2009-06-10 21:04:23 +0000
+++ disk/efi/efidisk.c 2009-12-29 09:04:06 +0000
@@ -440,7 +440,7 @@
for (d = fd_devices, count = 0; d; d = d->next, count++)
{
- grub_sprintf (buf, "fd%d", count);
+ grub_snprintf (buf, sizeof (buf), "fd%d", count);
grub_dprintf ("efidisk", "iterating %s\n", buf);
if (hook (buf))
return 1;
@@ -448,7 +448,7 @@
for (d = hd_devices, count = 0; d; d = d->next, count++)
{
- grub_sprintf (buf, "hd%d", count);
+ grub_snprintf (buf, sizeof (buf), "hd%d", count);
grub_dprintf ("efidisk", "iterating %s\n", buf);
if (hook (buf))
return 1;
@@ -456,7 +456,7 @@
for (d = cd_devices, count = 0; d; d = d->next, count++)
{
- grub_sprintf (buf, "cd%d", count);
+ grub_snprintf (buf, sizeof (buf), "cd%d", count);
grub_dprintf ("efidisk", "iterating %s\n", buf);
if (hook (buf))
return 1;
@@ -805,18 +805,10 @@
return 0;
}
- device_name = grub_malloc (grub_strlen (parent->name) + 1
- + grub_strlen (partition_name) + 1);
- if (! device_name)
- {
- grub_free (partition_name);
- grub_disk_close (parent);
- return 0;
- }
-
- grub_sprintf (device_name, "%s,%s", parent->name, partition_name);
+ device_name = grub_asprintf ("%s,%s", parent->name, partition_name);
grub_free (partition_name);
grub_disk_close (parent);
+
return device_name;
}
else
=== modified file 'disk/i386/pc/biosdisk.c'
--- disk/i386/pc/biosdisk.c 2009-12-10 18:15:20 +0000
+++ disk/i386/pc/biosdisk.c 2009-12-29 09:04:06 +0000
@@ -56,7 +56,8 @@
{
char name[10];
- grub_sprintf (name, (drive & 0x80) ? "hd%d" : "fd%d", drive & (~0x80));
+ grub_snprintf (name, sizeof (name),
+ (drive & 0x80) ? "hd%d" : "fd%d", drive & (~0x80));
return hook (name);
}
=== modified file 'disk/raid.c'
--- disk/raid.c 2009-12-24 22:53:05 +0000
+++ disk/raid.c 2009-12-29 09:04:06 +0000
@@ -556,7 +556,7 @@
}
}
- array->name = grub_malloc (13);
+ array->name = grub_asprintf ("md%d", array->number);
if (! array->name)
{
grub_free (array->uuid);
@@ -565,8 +565,6 @@
return grub_errno;
}
- grub_sprintf (array->name, "md%d", array->number);
-
grub_dprintf ("raid", "Found array %s (%s)\n", array->name,
scanner_name);
=== modified file 'disk/scsi.c'
--- disk/scsi.c 2009-12-24 22:53:05 +0000
+++ disk/scsi.c 2009-12-29 09:04:06 +0000
@@ -197,7 +197,6 @@
int scsi_iterate (const char *name, int luns)
{
- char sname[40];
int i;
/* In case of a single LUN, just return `usbX'. */
@@ -208,9 +207,13 @@
distinguish it. */
for (i = 0; i < luns; i++)
{
- grub_sprintf (sname, "%s%c", name, 'a' + i);
+ char *sname;
+ sname = grub_asprintf ("%s%c", name, 'a' + i);
+ if (!sname)
+ return 1;
if (hook (sname))
return 1;
+ grub_free (sname);
}
return 0;
}
=== modified file 'disk/usbms.c'
--- disk/usbms.c 2009-07-19 13:59:21 +0000
+++ disk/usbms.c 2009-12-29 09:04:06 +0000
@@ -200,11 +200,15 @@
for (p = grub_usbms_dev_list; p; p = p->next)
{
- char devname[20];
- grub_sprintf (devname, "usb%d", cnt);
+ char *devname;
+ devname = grub_asprintf ("usb%d", cnt);
if (hook (devname, p->luns))
- return 1;
+ {
+ grub_free (devname);
+ return 1;
+ }
+ grub_free (devname);
cnt++;
}
=== modified file 'efiemu/main.c'
--- efiemu/main.c 2009-12-26 10:01:33 +0000
+++ efiemu/main.c 2009-12-29 09:04:06 +0000
@@ -255,12 +255,11 @@
suffix = grub_efiemu_get_default_core_name ();
- filename = grub_malloc (grub_strlen (prefix) + grub_strlen (suffix) + 2);
+ filename = grub_asprintf ("%s/%s", prefix, suffix);
if (! filename)
return grub_error (GRUB_ERR_OUT_OF_MEMORY,
"couldn't allocate temporary space");
- grub_sprintf (filename, "%s/%s", prefix, suffix);
err = grub_efiemu_load_file (filename);
grub_free (filename);
=== modified file 'fs/ext2.c'
--- fs/ext2.c 2009-10-25 15:23:48 +0000
+++ fs/ext2.c 2009-12-29 09:04:06 +0000
@@ -875,12 +875,15 @@
data = grub_ext2_mount (disk);
if (data)
{
- *uuid = grub_malloc (40 + sizeof ('\0'));
- grub_sprintf (*uuid, "%04x%04x-%04x-%04x-%04x-%04x%04x%04x",
- grub_be_to_cpu16 (data->sblock.uuid[0]), grub_be_to_cpu16 (data->sblock.uuid[1]),
- grub_be_to_cpu16 (data->sblock.uuid[2]), grub_be_to_cpu16 (data->sblock.uuid[3]),
- grub_be_to_cpu16 (data->sblock.uuid[4]), grub_be_to_cpu16 (data->sblock.uuid[5]),
- grub_be_to_cpu16 (data->sblock.uuid[6]), grub_be_to_cpu16 (data->sblock.uuid[7]));
+ *uuid = grub_asprintf ("%04x%04x-%04x-%04x-%04x-%04x%04x%04x",
+ grub_be_to_cpu16 (data->sblock.uuid[0]),
+ grub_be_to_cpu16 (data->sblock.uuid[1]),
+ grub_be_to_cpu16 (data->sblock.uuid[2]),
+ grub_be_to_cpu16 (data->sblock.uuid[3]),
+ grub_be_to_cpu16 (data->sblock.uuid[4]),
+ grub_be_to_cpu16 (data->sblock.uuid[5]),
+ grub_be_to_cpu16 (data->sblock.uuid[6]),
+ grub_be_to_cpu16 (data->sblock.uuid[7]));
}
else
*uuid = NULL;
=== modified file 'fs/fat.c'
--- fs/fat.c 2009-12-25 00:04:51 +0000
+++ fs/fat.c 2009-12-29 09:04:06 +0000
@@ -833,9 +833,9 @@
data = grub_fat_mount (disk);
if (data)
{
- *uuid = grub_malloc (sizeof ("xxxx-xxxx"));
- grub_sprintf (*uuid, "%04x-%04x", (grub_uint16_t) (data->uuid >> 16),
- (grub_uint16_t) data->uuid);
+ *uuid = grub_asprintf ("%04x-%04x",
+ (grub_uint16_t) (data->uuid >> 16),
+ (grub_uint16_t) data->uuid);
}
else
*uuid = NULL;
=== modified file 'fs/hfs.c'
--- fs/hfs.c 2009-12-25 00:04:51 +0000
+++ fs/hfs.c 2009-12-29 09:04:06 +0000
@@ -1082,10 +1082,9 @@
data = grub_hfs_mount (device->disk);
if (data && data->sblock.num_serial != 0)
{
- *uuid = grub_malloc (16 + sizeof ('\0'));
- grub_sprintf (*uuid, "%016llx",
- (unsigned long long)
- grub_be_to_cpu64 (data->sblock.num_serial));
+ *uuid = grub_asprintf ("%016llx",
+ (unsigned long long)
+ grub_be_to_cpu64 (data->sblock.num_serial));
}
else
*uuid = NULL;
=== modified file 'fs/hfsplus.c'
--- fs/hfsplus.c 2009-12-25 00:04:51 +0000
+++ fs/hfsplus.c 2009-12-29 09:04:06 +0000
@@ -995,10 +995,9 @@
data = grub_hfsplus_mount (disk);
if (data)
{
- *uuid = grub_malloc (16 + sizeof ('\0'));
- grub_sprintf (*uuid, "%016llx",
- (unsigned long long)
- grub_be_to_cpu64 (data->volheader.num_serial));
+ *uuid = grub_asprintf ("%016llx",
+ (unsigned long long)
+ grub_be_to_cpu64 (data->volheader.num_serial));
}
else
*uuid = NULL;
=== modified file 'fs/i386/pc/pxe.c'
--- fs/i386/pc/pxe.c 2009-12-25 21:01:06 +0000
+++ fs/i386/pc/pxe.c 2009-12-29 09:04:06 +0000
@@ -356,7 +356,8 @@
for (i = 0; i < mac_len; i++)
{
- grub_sprintf (ptr, "%02x:", mac_addr[i] & 0xff);
+ grub_snprintf (ptr, sizeof (buf) - (ptr - buf),
+ "%02x:", mac_addr[i] & 0xff);
ptr += (sizeof ("XX:") - 1);
}
if (mac_len)
@@ -483,8 +484,8 @@
{
char buf[sizeof ("XXX.XXX.XXX.XXX")];
- grub_sprintf (buf, "%d.%d.%d.%d", (ip & 0xff),
- (ip >> 8) & 0xff, (ip >> 16) & 0xff, (ip >> 24) & 0xff);
+ grub_snprintf (buf, sizeof (buf), "%d.%d.%d.%d", (ip & 0xff),
+ (ip >> 8) & 0xff, (ip >> 16) & 0xff, (ip >> 24) & 0xff);
grub_env_set (varname, buf);
}
@@ -500,15 +501,13 @@
return 0;
/* Normalize the IP. */
- buf = grub_malloc (sizeof ("XXX.XXX.XXX.XXX"));
+ buf = grub_asprintf ("%d.%d.%d.%d", (newip & 0xff), (newip >> 8) & 0xff,
+ (newip >> 16) & 0xff, (newip >> 24) & 0xff);
if (!buf)
return 0;
*ip = newip;
- grub_sprintf (buf, "%d.%d.%d.%d", (newip & 0xff), (newip >> 8) & 0xff,
- (newip >> 16) & 0xff, (newip >> 24) & 0xff);
-
return buf;
}
@@ -544,11 +543,10 @@
else if (size > GRUB_PXE_MAX_BLKSIZE)
size = GRUB_PXE_MAX_BLKSIZE;
- buf = grub_malloc (sizeof ("XXXXXX XXXXXX"));
+ buf = grub_asprintf ("%d", size);
if (!buf)
return 0;
- grub_sprintf (buf, "%d", size);
grub_pxe_blksize = size;
return buf;
@@ -562,12 +560,10 @@
{
char *buf;
- buf = grub_malloc (sizeof ("XXXXXX XXXXXX"));
+ buf = grub_asprintf ("%d", grub_pxe_blksize);
if (buf)
- {
- grub_sprintf (buf, "%d", grub_pxe_blksize);
- grub_env_set ("net_pxe_blksize", buf);
- }
+ grub_env_set ("net_pxe_blksize", buf);
+ grub_free (buf);
set_ip_env ("pxe_default_server", grub_pxe_default_server_ip);
set_ip_env ("pxe_default_gateway", grub_pxe_default_gateway_ip);
=== modified file 'fs/iso9660.c'
--- fs/iso9660.c 2009-12-25 00:04:51 +0000
+++ fs/iso9660.c 2009-12-29 09:04:06 +0000
@@ -837,16 +837,23 @@
}
else
{
- *uuid = grub_malloc (sizeof ("YYYY-MM-DD-HH-mm-ss-hh"));
- grub_sprintf (*uuid, "%c%c%c%c-%c%c-%c%c-%c%c-%c%c-%c%c-%c%c",
- data->voldesc.modified.year[0], data->voldesc.modified.year[1],
- data->voldesc.modified.year[2], data->voldesc.modified.year[3],
- data->voldesc.modified.month[0], data->voldesc.modified.month[1],
- data->voldesc.modified.day[0], data->voldesc.modified.day[1],
- data->voldesc.modified.hour[0], data->voldesc.modified.hour[1],
- data->voldesc.modified.minute[0], data->voldesc.modified.minute[1],
- data->voldesc.modified.second[0], data->voldesc.modified.second[1],
- data->voldesc.modified.hundredth[0], data->voldesc.modified.hundredth[1]);
+ *uuid = grub_asprintf ("%c%c%c%c-%c%c-%c%c-%c%c-%c%c-%c%c-%c%c",
+ data->voldesc.modified.year[0],
+ data->voldesc.modified.year[1],
+ data->voldesc.modified.year[2],
+ data->voldesc.modified.year[3],
+ data->voldesc.modified.month[0],
+ data->voldesc.modified.month[1],
+ data->voldesc.modified.day[0],
+ data->voldesc.modified.day[1],
+ data->voldesc.modified.hour[0],
+ data->voldesc.modified.hour[1],
+ data->voldesc.modified.minute[0],
+ data->voldesc.modified.minute[1],
+ data->voldesc.modified.second[0],
+ data->voldesc.modified.second[1],
+ data->voldesc.modified.hundredth[0],
+ data->voldesc.modified.hundredth[1]);
}
}
else
=== modified file 'fs/jfs.c'
--- fs/jfs.c 2009-12-25 00:04:51 +0000
+++ fs/jfs.c 2009-12-29 09:04:06 +0000
@@ -842,17 +842,16 @@
data = grub_jfs_mount (disk);
if (data)
{
- *uuid = grub_malloc (40 + sizeof ('\0'));
-
- grub_sprintf (*uuid, "%02x%02x%02x%02x-%02x%02x-%02x%02x-%02x%02x-%02x%02x%02x%02x%02x%02x",
- data->sblock.uuid[0], data->sblock.uuid[1],
- data->sblock.uuid[2], data->sblock.uuid[3],
- data->sblock.uuid[4], data->sblock.uuid[5],
- data->sblock.uuid[6], data->sblock.uuid[7],
- data->sblock.uuid[8], data->sblock.uuid[9],
- data->sblock.uuid[10], data->sblock.uuid[11],
- data->sblock.uuid[12], data->sblock.uuid[13],
- data->sblock.uuid[14], data->sblock.uuid[15]);
+ *uuid = grub_asprintf ("%02x%02x%02x%02x-%02x%02x-%02x%02x-%02x%02x-"
+ "%02x%02x%02x%02x%02x%02x",
+ data->sblock.uuid[0], data->sblock.uuid[1],
+ data->sblock.uuid[2], data->sblock.uuid[3],
+ data->sblock.uuid[4], data->sblock.uuid[5],
+ data->sblock.uuid[6], data->sblock.uuid[7],
+ data->sblock.uuid[8], data->sblock.uuid[9],
+ data->sblock.uuid[10], data->sblock.uuid[11],
+ data->sblock.uuid[12], data->sblock.uuid[13],
+ data->sblock.uuid[14], data->sblock.uuid[15]);
}
else
*uuid = NULL;
=== modified file 'fs/ntfs.c'
--- fs/ntfs.c 2009-12-24 22:53:05 +0000
+++ fs/ntfs.c 2009-12-29 09:04:06 +0000
@@ -1072,8 +1072,7 @@
data = grub_ntfs_mount (disk);
if (data)
{
- *uuid = grub_malloc (16 + sizeof ('\0'));
- grub_sprintf (*uuid, "%016llx", (unsigned long long) data->uuid);
+ *uuid = grub_asprintf ("%016llx", (unsigned long long) data->uuid);
}
else
*uuid = NULL;
=== modified file 'fs/reiserfs.c'
--- fs/reiserfs.c 2009-12-25 00:04:51 +0000
+++ fs/reiserfs.c 2009-12-29 09:04:06 +0000
@@ -1335,12 +1335,15 @@
data = grub_reiserfs_mount (disk);
if (data)
{
- *uuid = grub_malloc (sizeof ("xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx"));
- grub_sprintf (*uuid, "%04x%04x-%04x-%04x-%04x-%04x%04x%04x",
- grub_be_to_cpu16 (data->superblock.uuid[0]), grub_be_to_cpu16 (data->superblock.uuid[1]),
- grub_be_to_cpu16 (data->superblock.uuid[2]), grub_be_to_cpu16 (data->superblock.uuid[3]),
- grub_be_to_cpu16 (data->superblock.uuid[4]), grub_be_to_cpu16 (data->superblock.uuid[5]),
- grub_be_to_cpu16 (data->superblock.uuid[6]), grub_be_to_cpu16 (data->superblock.uuid[7]));
+ *uuid = grub_asprintf ("%04x%04x-%04x-%04x-%04x-%04x%04x%04x",
+ grub_be_to_cpu16 (data->superblock.uuid[0]),
+ grub_be_to_cpu16 (data->superblock.uuid[1]),
+ grub_be_to_cpu16 (data->superblock.uuid[2]),
+ grub_be_to_cpu16 (data->superblock.uuid[3]),
+ grub_be_to_cpu16 (data->superblock.uuid[4]),
+ grub_be_to_cpu16 (data->superblock.uuid[5]),
+ grub_be_to_cpu16 (data->superblock.uuid[6]),
+ grub_be_to_cpu16 (data->superblock.uuid[7]));
}
else
*uuid = NULL;
=== modified file 'fs/ufs.c'
--- fs/ufs.c 2009-12-25 00:04:51 +0000
+++ fs/ufs.c 2009-12-29 09:04:06 +0000
@@ -732,12 +732,9 @@
data = grub_ufs_mount (disk);
if (data && (data->sblock.uuidhi != 0 || data->sblock.uuidlow != 0))
- {
- *uuid = grub_malloc (16 + sizeof ('\0'));
- grub_sprintf (*uuid, "%08x%08x",
- (unsigned) grub_le_to_cpu32 (data->sblock.uuidhi),
- (unsigned) grub_le_to_cpu32 (data->sblock.uuidlow));
- }
+ *uuid = grub_asprintf ("%08x%08x",
+ (unsigned) grub_le_to_cpu32 (data->sblock.uuidhi),
+ (unsigned) grub_le_to_cpu32 (data->sblock.uuidlow));
else
*uuid = NULL;
=== modified file 'fs/xfs.c'
--- fs/xfs.c 2009-12-25 00:04:51 +0000
+++ fs/xfs.c 2009-12-29 09:04:06 +0000
@@ -777,12 +777,15 @@
data = grub_xfs_mount (disk);
if (data)
{
- *uuid = grub_malloc (sizeof ("xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx"));
- grub_sprintf (*uuid, "%04x%04x-%04x-%04x-%04x-%04x%04x%04x",
- grub_be_to_cpu16 (data->sblock.uuid[0]), grub_be_to_cpu16 (data->sblock.uuid[1]),
- grub_be_to_cpu16 (data->sblock.uuid[2]), grub_be_to_cpu16 (data->sblock.uuid[3]),
- grub_be_to_cpu16 (data->sblock.uuid[4]), grub_be_to_cpu16 (data->sblock.uuid[5]),
- grub_be_to_cpu16 (data->sblock.uuid[6]), grub_be_to_cpu16 (data->sblock.uuid[7]));
+ *uuid = grub_asprintf ("%04x%04x-%04x-%04x-%04x-%04x%04x%04x",
+ grub_be_to_cpu16 (data->sblock.uuid[0]),
+ grub_be_to_cpu16 (data->sblock.uuid[1]),
+ grub_be_to_cpu16 (data->sblock.uuid[2]),
+ grub_be_to_cpu16 (data->sblock.uuid[3]),
+ grub_be_to_cpu16 (data->sblock.uuid[4]),
+ grub_be_to_cpu16 (data->sblock.uuid[5]),
+ grub_be_to_cpu16 (data->sblock.uuid[6]),
+ grub_be_to_cpu16 (data->sblock.uuid[7]));
}
else
*uuid = NULL;
=== modified file 'gettext/gettext.c'
--- gettext/gettext.c 2009-12-25 23:50:59 +0000
+++ gettext/gettext.c 2009-12-29 09:04:06 +0000
@@ -275,14 +275,19 @@
/* Warning: if changing some paths in the below line, change the grub_malloc
contents below. */
- grub_sprintf (mo_file, "%s/%s.mo", locale_dir, lang);
+ mo_file = grub_asprintf ("%s/%s.mo", locale_dir, lang);
+ if (!mo_file)
+ return;
fd_mo = grub_mofile_open (mo_file);
/* Will try adding .gz as well. */
if (fd_mo == NULL)
{
- grub_sprintf (mo_file, "%s.gz", mo_file);
+ grub_free (mo_file);
+ mo_file = grub_asprintf ("%s.gz", mo_file);
+ if (!mo_file)
+ return;
fd_mo = grub_mofile_open (mo_file);
}
=== modified file 'hook/datehook.c'
--- hook/datehook.c 2009-05-04 03:49:08 +0000
+++ hook/datehook.c 2009-12-29 09:04:06 +0000
@@ -76,7 +76,7 @@
return grub_get_weekday_name (&datetime);
}
- grub_sprintf (buf, "%d", n);
+ grub_snprintf (buf, sizeof (buf), "%d", n);
break;
}
}
=== modified file 'include/grub/misc.h'
--- include/grub/misc.h 2009-12-18 02:57:32 +0000
+++ include/grub/misc.h 2009-12-29 09:04:06 +0000
@@ -179,8 +179,13 @@
const char *condition,
const char *fmt, ...) __attribute__ ((format (printf, 4, 5)));
int EXPORT_FUNC(grub_vprintf) (const char *fmt, va_list args);
-int EXPORT_FUNC(grub_sprintf) (char *str, const char *fmt, ...) __attribute__ ((format (printf, 2, 3)));
-int EXPORT_FUNC(grub_vsprintf) (char *str, const char *fmt, va_list args);
+int EXPORT_FUNC(grub_snprintf) (char *str, grub_size_t n, const char *fmt, ...)
+ __attribute__ ((format (printf, 3, 4)));
+int EXPORT_FUNC(grub_vsnprintf) (char *str, grub_size_t n, const char *fmt,
+ va_list args);
+char *EXPORT_FUNC(grub_asprintf) (const char *fmt, ...)
+ __attribute__ ((format (printf, 1, 2)));
+char *EXPORT_FUNC(grub_avsprintf) (const char *fmt, va_list args);
void EXPORT_FUNC(grub_exit) (void) __attribute__ ((noreturn));
void EXPORT_FUNC(grub_abort) (void) __attribute__ ((noreturn));
grub_ssize_t EXPORT_FUNC(grub_utf8_to_ucs4) (grub_uint32_t *dest,
=== modified file 'kern/device.c'
--- kern/device.c 2009-12-10 12:26:22 +0000
+++ kern/device.c 2009-12-29 09:04:06 +0000
@@ -86,7 +86,7 @@
struct part_ent
{
struct part_ent *next;
- char name[0];
+ char *name;
} *ents;
int iterate_disk (const char *disk_name)
@@ -118,6 +118,7 @@
if (!ret)
ret = hook (p->name);
+ grub_free (p->name);
grub_free (p);
p = next;
}
@@ -138,15 +139,20 @@
if (! partition_name)
return 1;
- p = grub_malloc (sizeof (p->next) + grub_strlen (disk->name) + 1 +
- grub_strlen (partition_name) + 1);
+ p = grub_malloc (sizeof (p->next));
if (!p)
{
grub_free (partition_name);
return 1;
}
- grub_sprintf (p->name, "%s,%s", disk->name, partition_name);
+ p->name = grub_asprintf ("%s,%s", disk->name, partition_name);
+ if (!p->name)
+ {
+ grub_free (partition_name);
+ grub_free (p);
+ return 1;
+ }
grub_free (partition_name);
p->next = ents;
=== modified file 'kern/dl.c'
--- kern/dl.c 2009-11-23 15:31:54 +0000
+++ kern/dl.c 2009-12-29 09:04:06 +0000
@@ -628,12 +628,10 @@
return 0;
}
- filename = (char *) grub_malloc (grub_strlen (grub_dl_dir) + 1
- + grub_strlen (name) + 4 + 1);
+ filename = grub_asprintf ("%s/%s.mod", grub_dl_dir, name);
if (! filename)
return 0;
- grub_sprintf (filename, "%s/%s.mod", grub_dl_dir, name);
mod = grub_dl_load_file (filename);
grub_free (filename);
=== modified file 'kern/efi/init.c'
--- kern/efi/init.c 2009-06-10 21:04:23 +0000
+++ kern/efi/init.c 2009-12-29 09:04:06 +0000
@@ -63,11 +63,10 @@
if (p)
*p = '\0';
- prefix = grub_malloc (1 + grub_strlen (device) + 1
- + grub_strlen (file) + 1);
+ prefix = grub_asprintf ("(%s)%s", device, file);
if (prefix)
{
- grub_sprintf (prefix, "(%s)%s", device, file);
+
grub_env_set ("prefix", prefix);
grub_free (prefix);
}
=== modified file 'kern/env.c'
--- kern/env.c 2009-12-23 16:41:32 +0000
+++ kern/env.c 2009-12-29 09:04:06 +0000
@@ -356,14 +356,7 @@
static char *
mangle_data_slot_name (const char *name)
{
- char *mangled_name;
-
- mangled_name = grub_malloc (grub_strlen (name) + 2);
- if (! mangled_name)
- return 0;
-
- grub_sprintf (mangled_name, "\e%s", name);
- return mangled_name;
+ return grub_asprintf ("\e%s", name);
}
grub_err_t
=== modified file 'kern/err.c'
--- kern/err.c 2009-06-10 21:04:23 +0000
+++ kern/err.c 2009-12-29 09:04:06 +0000
@@ -44,7 +44,7 @@
grub_errno = n;
va_start (ap, fmt);
- grub_vsprintf (grub_errmsg, fmt, ap);
+ grub_vsnprintf (grub_errmsg, sizeof (grub_errmsg), fmt, ap);
va_end (ap);
return n;
=== modified file 'kern/i386/pc/init.c'
--- kern/i386/pc/init.c 2009-06-15 23:25:38 +0000
+++ kern/i386/pc/init.c 2009-12-29 09:04:06 +0000
@@ -56,22 +56,30 @@
make_install_device (void)
{
/* XXX: This should be enough. */
- char dev[100];
+ char dev[100], *ptr = dev;
if (grub_prefix[0] != '(')
{
/* No hardcoded root partition - make it from the boot drive and the
partition number encoded at the install time. */
- grub_sprintf (dev, "(%cd%u", (grub_boot_drive & 0x80) ? 'h' : 'f',
+ grub_snprintf (dev, sizeof (dev),
+ "(%cd%u", (grub_boot_drive & 0x80) ? 'h' : 'f',
grub_boot_drive & 0x7f);
+ ptr += grub_strlen (ptr);
if (grub_install_dos_part >= 0)
- grub_sprintf (dev + grub_strlen (dev), ",%u", grub_install_dos_part + 1);
+ grub_snprintf (ptr, sizeof (dev) - (ptr - dev),
+ ",%u", grub_install_dos_part + 1);
+
+ ptr += grub_strlen (ptr);
if (grub_install_bsd_part >= 0)
- grub_sprintf (dev + grub_strlen (dev), ",%c", grub_install_bsd_part + 'a');
-
- grub_sprintf (dev + grub_strlen (dev), ")%s", grub_prefix);
+ grub_snprintf (ptr, sizeof (dev) - (ptr - dev), ",%c",
+ grub_install_bsd_part + 'a');
+
+ ptr += grub_strlen (ptr);
+
+ grub_snprintf (ptr, sizeof (dev) - (ptr - dev), ")%s", grub_prefix);
grub_strcpy (grub_prefix, dev);
}
=== modified file 'kern/ieee1275/init.c'
--- kern/ieee1275/init.c 2009-12-24 22:53:05 +0000
+++ kern/ieee1275/init.c 2009-12-29 09:04:06 +0000
@@ -111,11 +111,12 @@
*lastslash = '\0';
grub_translate_ieee1275_path (filename);
- newprefix = grub_malloc (grub_strlen (prefix)
- + grub_strlen (filename));
- grub_sprintf (newprefix, "%s%s", prefix, filename);
- grub_free (prefix);
- prefix = newprefix;
+ newprefix = grub_asprintf ("%s%s", prefix, filename);
+ if (newprefix)
+ {
+ grub_free (prefix);
+ prefix = newprefix;
+ }
}
}
=== modified file 'kern/ieee1275/openfw.c'
--- kern/ieee1275/openfw.c 2009-12-24 22:53:05 +0000
+++ kern/ieee1275/openfw.c 2009-12-29 09:04:06 +0000
@@ -38,7 +38,7 @@
grub_ieee1275_phandle_t dev;
grub_ieee1275_phandle_t child;
char *childtype, *childpath;
- char *childname, *fullname;
+ char *childname;
int ret = 0;
if (grub_ieee1275_finddevice (devpath, &dev))
@@ -63,19 +63,12 @@
grub_free (childtype);
return 0;
}
- fullname = grub_malloc (IEEE1275_MAX_PATH_LEN);
- if (!fullname)
- {
- grub_free (childname);
- grub_free (childpath);
- grub_free (childtype);
- return 0;
- }
do
{
struct grub_ieee1275_devalias alias;
grub_ssize_t actual;
+ char *fullname;
if (grub_ieee1275_get_property (child, "device_type", childtype,
IEEE1275_MAX_PROP_LEN, &actual))
@@ -89,18 +82,25 @@
IEEE1275_MAX_PROP_LEN, &actual))
continue;
- grub_sprintf (fullname, "%s/%s", devpath, childname);
+ fullname = grub_asprintf ("%s/%s", devpath, childname);
+ if (!fullname)
+ {
+ grub_free (childname);
+ grub_free (childpath);
+ grub_free (childtype);
+ return 0;
+ }
alias.type = childtype;
alias.path = childpath;
alias.name = fullname;
ret = hook (&alias);
+ grub_free (fullname);
if (ret)
break;
}
while (grub_ieee1275_peer (child, &child));
- grub_free (fullname);
grub_free (childname);
grub_free (childpath);
grub_free (childtype);
@@ -330,12 +330,11 @@
{
char *filepath = comma + 1;
- ret = grub_malloc (grub_strlen (filepath) + 1);
/* Make sure filepath has leading backslash. */
if (filepath[0] != '\\')
- grub_sprintf (ret, "\\%s", filepath);
+ ret = grub_asprintf ("\\%s", filepath);
else
- grub_strcpy (ret, filepath);
+ ret = grub_strdup (filepath);
}
}
else if (ptype == GRUB_PARSE_PARTITION)
@@ -383,15 +382,10 @@
/* GRUB partition 1 is OF partition 0. */
partno++;
- /* Assume partno will require less than five bytes to encode. */
- encoding = grub_malloc (grub_strlen (device) + 3 + 5);
- grub_sprintf (encoding, "(%s,%d)", device, partno);
+ encoding = grub_asprintf ("(%s,%d)", device, partno);
}
else
- {
- encoding = grub_malloc (grub_strlen (device) + 2);
- grub_sprintf (encoding, "(%s)", device);
- }
+ encoding = grub_asprintf ("(%s)", device);
grub_free (partition);
grub_free (device);
=== modified file 'kern/misc.c'
--- kern/misc.c 2009-12-18 02:57:32 +0000
+++ kern/misc.c 2009-12-29 09:04:06 +0000
@@ -26,6 +26,9 @@
#include <grub/i18n.h>
static int
+grub_vsnprintf_real (char *str, grub_size_t n, const char *fmt, va_list args);
+
+static int
grub_iswordseparator (int c)
{
return (grub_isspace (c) || c == ',' || c == ';' || c == '|' || c == '&');
@@ -202,7 +205,7 @@
{
int ret;
- ret = grub_vsprintf (0, fmt, args);
+ ret = grub_vsnprintf_real (0, 0, fmt, args);
grub_refresh ();
return ret;
}
@@ -626,11 +629,11 @@
return p;
}
-int
-grub_vsprintf (char *str, const char *fmt, va_list args)
+static int
+grub_vsnprintf_real (char *str, grub_size_t n, const char *fmt, va_list args)
{
char c;
- int count = 0;
+ grub_size_t count = 0;
auto void write_char (unsigned char ch);
auto void write_str (const char *s);
auto void write_fill (const char ch, int n);
@@ -638,7 +641,10 @@
void write_char (unsigned char ch)
{
if (str)
- *str++ = ch;
+ {
+ if (count < n)
+ *str++ = ch;
+ }
else
grub_putchar (ch);
@@ -867,13 +873,64 @@
}
int
-grub_sprintf (char *str, const char *fmt, ...)
+grub_vsnprintf (char *str, grub_size_t n, const char *fmt, va_list ap)
+{
+ grub_size_t ret;
+
+ if (!n)
+ return 0;
+
+ n--;
+
+ ret = grub_vsnprintf_real (str, n, fmt, ap);
+
+ return ret < n ? ret : n;
+}
+
+int
+grub_snprintf (char *str, grub_size_t n, const char *fmt, ...)
{
va_list ap;
int ret;
va_start (ap, fmt);
- ret = grub_vsprintf (str, fmt, ap);
+ ret = grub_vsnprintf (str, n, fmt, ap);
+ va_end (ap);
+
+ return ret;
+}
+
+#define PREALLOC_SIZE 255
+
+char *
+grub_avsprintf (const char *fmt, va_list ap)
+{
+ grub_size_t s, as = PREALLOC_SIZE;
+ char *ret;
+
+ while (1)
+ {
+ ret = grub_malloc (as + 1);
+ if (!ret)
+ return NULL;
+
+ s = grub_vsnprintf (ret, as, fmt, ap);
+ if (s <= as)
+ return ret;
+
+ grub_free (ret);
+ as = s;
+ }
+}
+
+char *
+grub_asprintf (const char *fmt, ...)
+{
+ va_list ap;
+ char *ret;
+
+ va_start (ap, fmt);
+ ret = grub_avsprintf (fmt, ap);
va_end (ap);
return ret;
=== modified file 'kern/sparc64/ieee1275/init.c'
--- kern/sparc64/ieee1275/init.c 2009-04-30 13:17:10 +0000
+++ kern/sparc64/ieee1275/init.c 2009-12-29 09:04:06 +0000
@@ -90,10 +90,7 @@
}
prefix = grub_ieee1275_encode_devname (bootpath);
- path = grub_malloc (grub_strlen (grub_prefix)
- + grub_strlen (prefix)
- + 2);
- grub_sprintf(path, "%s%s", prefix, grub_prefix);
+ path = grub_asprintf("%s%s", prefix, grub_prefix);
grub_strcpy (grub_prefix, path);
=== modified file 'lib/hexdump.c'
--- lib/hexdump.c 2009-02-02 19:43:14 +0000
+++ lib/hexdump.c 2009-12-29 09:04:06 +0000
@@ -31,21 +31,22 @@
{
int cnt, i;
- pos = grub_sprintf (line, "%08lx ", bse);
+ pos = grub_snprintf (line, sizeof (line), "%08lx ", bse);
cnt = 16;
if (cnt > len)
cnt = len;
for (i = 0; i < cnt; i++)
{
- pos += grub_sprintf (&line[pos], "%02x ", (unsigned char) buf[i]);
+ pos += grub_snprintf (&line[pos], sizeof (line) - pos,
+ "%02x ", (unsigned char) buf[i]);
if ((i & 7) == 7)
line[pos++] = ' ';
}
for (; i < 16; i++)
{
- pos += grub_sprintf (&line[pos], " ");
+ pos += grub_snprintf (&line[pos], sizeof (line) - pos, " ");
if ((i & 7) == 7)
line[pos++] = ' ';
}
=== modified file 'loader/i386/bsd.c'
--- loader/i386/bsd.c 2009-12-26 10:01:33 +0000
+++ loader/i386/bsd.c 2009-12-29 09:04:06 +0000
@@ -1138,14 +1138,20 @@
if (*curr)
{
- char name[grub_strlen (curr) + sizeof("kFreeBSD.")];
+ char *name;
if (*p == '"')
p++;
- grub_sprintf (name, "kFreeBSD.%s", curr);
+ name = grub_asprintf ("kFreeBSD.%s", curr);
+ if (!name)
+ goto fail;
if (grub_env_set (name, p))
- goto fail;
+ {
+ grub_free (name);
+ goto fail;
+ }
+ grub_free (name);
}
}
=== modified file 'loader/i386/linux.c'
--- loader/i386/linux.c 2009-12-26 23:36:59 +0000
+++ loader/i386/linux.c 2009-12-29 09:04:06 +0000
@@ -517,11 +517,9 @@
May change in future if we have modes without framebuffer. */
if (modevar && *modevar != 0)
{
- tmp = grub_malloc (grub_strlen (modevar)
- + sizeof (";text"));
+ tmp = grub_asprintf ("%s;text", modevar);
if (! tmp)
return grub_errno;
- grub_sprintf (tmp, "%s;text", modevar);
err = grub_video_set_mode (tmp, 0);
grub_free (tmp);
}
@@ -779,19 +777,18 @@
break;
}
- buf = grub_malloc (sizeof ("WWWWxHHHHxDD;WWWWxHHHH"));
- if (! buf)
- goto fail;
-
linux_mode
= &linux_vesafb_modes[vid_mode - GRUB_LINUX_VID_MODE_VESA_START];
- grub_sprintf (buf, "%ux%ux%u,%ux%u",
- linux_vesafb_res[linux_mode->res_index].width,
- linux_vesafb_res[linux_mode->res_index].height,
- linux_mode->depth,
- linux_vesafb_res[linux_mode->res_index].width,
- linux_vesafb_res[linux_mode->res_index].height);
+ buf = grub_asprintf ("%ux%ux%u,%ux%u",
+ linux_vesafb_res[linux_mode->res_index].width,
+ linux_vesafb_res[linux_mode->res_index].height,
+ linux_mode->depth,
+ linux_vesafb_res[linux_mode->res_index].width,
+ linux_vesafb_res[linux_mode->res_index].height);
+ if (! buf)
+ goto fail;
+
grub_printf ("%s is deprecated. "
"Use set gfxpayload=%s before "
"linux command instead.\n",
=== modified file 'loader/i386/pc/xnu.c'
--- loader/i386/pc/xnu.c 2009-10-15 12:40:13 +0000
+++ loader/i386/pc/xnu.c 2009-12-29 09:04:06 +0000
@@ -52,12 +52,10 @@
err = grub_video_set_mode (DEFAULT_VIDEO_MODE, video_hook);
else
{
- tmp = grub_malloc (grub_strlen (modevar)
- + sizeof (DEFAULT_VIDEO_MODE) + 1);
+ tmp = grub_asprintf ("%s;" DEFAULT_VIDEO_MODE, modevar);
if (! tmp)
return grub_error (GRUB_ERR_OUT_OF_MEMORY,
"couldn't allocate temporary storag");
- grub_sprintf (tmp, "%s;" DEFAULT_VIDEO_MODE, modevar);
err = grub_video_set_mode (tmp, video_hook);
grub_free (tmp);
}
=== modified file 'loader/i386/xnu.c'
--- loader/i386/xnu.c 2009-12-24 22:53:05 +0000
+++ loader/i386/xnu.c 2009-12-29 09:04:06 +0000
@@ -748,11 +748,13 @@
#endif
/* The name of key for new table. */
- grub_sprintf (guidbuf, "%08x-%04x-%04x-%02x%02x-",
- guid.data1, guid.data2, guid.data3, guid.data4[0],
- guid.data4[1]);
+ grub_snprintf (guidbuf, sizeof (guidbuf), "%08x-%04x-%04x-%02x%02x-",
+ guid.data1, guid.data2, guid.data3, guid.data4[0],
+ guid.data4[1]);
for (j = 2; j < 8; j++)
- grub_sprintf (guidbuf + grub_strlen (guidbuf), "%02x", guid.data4[j]);
+ grub_snprintf (guidbuf + grub_strlen (guidbuf),
+ sizeof (guidbuf) - grub_strlen (guidbuf),
+ "%02x", guid.data4[j]);
/* For some reason GUID has to be in uppercase. */
for (j = 0; guidbuf[j] ; j++)
if (guidbuf[j] >= 'a' && guidbuf[j] <= 'f')
=== modified file 'loader/xnu.c'
--- loader/xnu.c 2009-12-26 23:36:59 +0000
+++ loader/xnu.c 2009-12-29 09:04:06 +0000
@@ -568,10 +568,9 @@
return grub_error (GRUB_ERR_OUT_OF_MEMORY, "can't register memory");
if (suffix)
{
- driverkey->name = grub_malloc (grub_strlen (prefix) + 10);
+ driverkey->name = grub_asprintf ("%s%d", prefix, (*suffix)++);
if (!driverkey->name)
return grub_error (GRUB_ERR_OUT_OF_MEMORY, "can't register memory");
- grub_sprintf (driverkey->name, "%s%d", prefix, (*suffix)++);
}
else
driverkey->name = grub_strdup (prefix);
=== modified file 'normal/autofs.c'
--- normal/autofs.c 2009-11-09 14:45:28 +0000
+++ normal/autofs.c 2009-12-29 09:04:06 +0000
@@ -63,12 +63,11 @@
{
char *filename;
- filename = grub_malloc (grub_strlen (prefix) + sizeof ("/fs.lst"));
+ filename = grub_asprintf ("%s/fs.lst", prefix);
if (filename)
{
grub_file_t file;
- grub_sprintf (filename, "%s/fs.lst", prefix);
file = grub_file_open (filename);
if (file)
{
=== modified file 'normal/completion.c'
--- normal/completion.c 2009-12-20 21:54:12 +0000
+++ normal/completion.c 2009-12-29 09:04:06 +0000
@@ -107,16 +107,11 @@
if (! partition_name)
return 1;
- name = grub_malloc (grub_strlen (disk_name) + 1
- + grub_strlen (partition_name) + 1);
+ name = grub_asprintf ("%s,%s", disk_name, partition_name);
+ grub_free (partition_name);
+
if (! name)
- {
- grub_free (partition_name);
- return 1;
- }
-
- grub_sprintf (name, "%s,%s", disk_name, partition_name);
- grub_free (partition_name);
+ return 1;
ret = add_completion (name, ")", GRUB_COMPLETION_TYPE_PARTITION);
grub_free (name);
@@ -141,11 +136,15 @@
}
else if (grub_strcmp (filename, ".") && grub_strcmp (filename, ".."))
{
- char fname[grub_strlen (filename) + 2];
+ char *fname;
- grub_sprintf (fname, "%s/", filename);
+ fname = grub_asprintf ("%s/", filename);
if (add_completion (fname, "", GRUB_COMPLETION_TYPE_FILE))
- return 1;
+ {
+ grub_free (fname);
+ return 1;
+ }
+ grub_free (fname);
}
return 0;
@@ -360,8 +359,9 @@
if (!option->longarg)
continue;
- longarg = grub_malloc (grub_strlen (option->longarg));
- grub_sprintf (longarg, "--%s", option->longarg);
+ longarg = grub_asprintf ("--%s", option->longarg);
+ if (!longarg)
+ return 1;
if (add_completion (longarg, " ", GRUB_COMPLETION_TYPE_ARGUMENT))
{
=== modified file 'normal/dyncmd.c'
--- normal/dyncmd.c 2009-12-25 20:51:05 +0000
+++ normal/dyncmd.c 2009-12-29 09:04:06 +0000
@@ -75,12 +75,11 @@
{
char *filename;
- filename = grub_malloc (grub_strlen (prefix) + sizeof ("/command.lst"));
+ filename = grub_asprintf ("%s/command.lst", prefix);
if (filename)
{
grub_file_t file;
- grub_sprintf (filename, "%s/command.lst", prefix);
file = grub_file_open (filename);
if (file)
{
=== modified file 'normal/handler.c'
--- normal/handler.c 2009-12-26 10:01:33 +0000
+++ normal/handler.c 2009-12-29 09:04:06 +0000
@@ -172,12 +172,11 @@
{
char *filename;
- filename = grub_malloc (grub_strlen (prefix) + sizeof ("/handler.lst"));
+ filename = grub_asprintf ("%s/handler.lst", prefix);
if (filename)
{
grub_file_t file;
- grub_sprintf (filename, "%s/handler.lst", prefix);
file = grub_file_open (filename);
if (file)
{
=== modified file 'normal/main.c'
--- normal/main.c 2009-12-26 23:43:21 +0000
+++ normal/main.c 2009-12-29 09:04:06 +0000
@@ -389,16 +389,17 @@
int posx;
const char *msg = _("GNU GRUB version %s");
- char *msg_formatted = grub_malloc (grub_strlen(msg) +
- grub_strlen(PACKAGE_VERSION));
-
- grub_cls ();
-
- grub_sprintf (msg_formatted, msg, PACKAGE_VERSION);
+ char *msg_formatted;
grub_uint32_t *unicode_msg;
grub_uint32_t *last_position;
+ grub_cls ();
+
+ msg_formatted = grub_asprintf (msg, PACKAGE_VERSION);
+ if (!msg_formatted)
+ return;
+
msg_len = grub_utf8_to_ucs4_alloc (msg_formatted,
&unicode_msg, &last_position);
@@ -475,11 +476,10 @@
prefix = grub_env_get ("prefix");
if (prefix)
{
- config = grub_malloc (grub_strlen (prefix) + sizeof ("/grub.cfg"));
+ config = grub_asprintf ("%s/grub.cfg", prefix);
if (! config)
goto quit;
- grub_sprintf (config, "%s/grub.cfg", prefix);
grub_enter_normal_mode (config);
grub_free (config);
}
@@ -528,10 +528,11 @@
const char *msg_esc = _("ESC at any time exits.");
- char *msg_formatted = grub_malloc (sizeof (char) * (grub_strlen (msg) +
- grub_strlen(msg_esc) + 1));
+ char *msg_formatted;
- grub_sprintf (msg_formatted, msg, reader_nested ? msg_esc : "");
+ msg_formatted = grub_asprintf (msg, reader_nested ? msg_esc : "");
+ if (!msg_formatted)
+ return grub_errno;
grub_print_message_indented (msg_formatted, 3, STANDARD_MARGIN);
grub_puts ("\n");
@@ -546,9 +547,11 @@
grub_normal_read_line (char **line, int cont)
{
grub_parser_t parser = grub_parser_get_current ();
- char prompt[sizeof(">") + grub_strlen (parser->name)];
+ char *prompt;
- grub_sprintf (prompt, "%s>", parser->name);
+ prompt = grub_asprintf ("%s>", parser->name);
+ if (!prompt)
+ return grub_errno;
while (1)
{
=== modified file 'normal/menu.c'
--- normal/menu.c 2009-08-24 23:55:06 +0000
+++ normal/menu.c 2009-12-29 09:04:06 +0000
@@ -78,7 +78,7 @@
{
char buf[16];
- grub_sprintf (buf, "%d", timeout);
+ grub_snprintf (buf, sizeof (buf), "%d", timeout);
grub_env_set ("timeout", buf);
}
}
=== modified file 'normal/menu_text.c'
--- normal/menu_text.c 2009-12-27 21:32:52 +0000
+++ normal/menu_text.c 2009-12-29 09:04:06 +0000
@@ -210,13 +210,14 @@
}
else
{
- const char *msg = _("Use the %C and %C keys to select which \
-entry is highlighted.\n");
- char *msg_translated =
- grub_malloc (sizeof (char) * grub_strlen (msg) + 1);
+ const char *msg = _("Use the %C and %C keys to select which "
+ "entry is highlighted.\n");
+ char *msg_translated;
- grub_sprintf (msg_translated, msg, (grub_uint32_t) GRUB_TERM_DISP_UP,
- (grub_uint32_t) GRUB_TERM_DISP_DOWN);
+ msg_translated = grub_asprintf (msg, (grub_uint32_t) GRUB_TERM_DISP_UP,
+ (grub_uint32_t) GRUB_TERM_DISP_DOWN);
+ if (!msg_translated)
+ return;
grub_putchar ('\n');
grub_print_message_indented (msg_translated, STANDARD_MARGIN, STANDARD_MARGIN);
@@ -394,13 +395,13 @@
{
const char *msg =
_("The highlighted entry will be booted automatically in %ds.");
+ char *msg_translated;
grub_gotoxy (0, GRUB_TERM_HEIGHT - 3);
- char *msg_translated =
- grub_malloc (sizeof (char) * grub_strlen (msg) + 5);
-
- grub_sprintf (msg_translated, msg, timeout);
+ msg_translated = grub_asprintf (msg, timeout);
+ if (!msg_translated)
+ return;
grub_print_message_indented (msg_translated, 3, 0);
int posx;
=== modified file 'partmap/acorn.c'
--- partmap/acorn.c 2009-12-24 22:53:05 +0000
+++ partmap/acorn.c 2009-12-29 09:04:06 +0000
@@ -175,14 +175,7 @@
static char *
acorn_partition_map_get_name (const grub_partition_t p)
{
- char *name;
-
- name = grub_malloc (13);
- if (! name)
- return 0;
-
- grub_sprintf (name, "%d", p->index + 1);
- return name;
+ return grub_asprintf ("%d", p->index + 1);
}
\f
=== modified file 'partmap/amiga.c'
--- partmap/amiga.c 2009-12-24 22:53:05 +0000
+++ partmap/amiga.c 2009-12-29 09:04:06 +0000
@@ -184,14 +184,7 @@
static char *
amiga_partition_map_get_name (const grub_partition_t p)
{
- char *name;
-
- name = grub_malloc (13);
- if (! name)
- return 0;
-
- grub_sprintf (name, "%d", p->index + 1);
- return name;
+ return grub_asprintf ("%d", p->index + 1);
}
\f
=== modified file 'partmap/apple.c'
--- partmap/apple.c 2009-12-24 22:53:05 +0000
+++ partmap/apple.c 2009-12-29 09:04:06 +0000
@@ -227,14 +227,7 @@
static char *
apple_partition_map_get_name (const grub_partition_t p)
{
- char *name;
-
- name = grub_malloc (13);
- if (! name)
- return 0;
-
- grub_sprintf (name, "%d", p->index + 1);
- return name;
+ return grub_asprintf ("%d", p->index + 1);
}
\f
=== modified file 'partmap/gpt.c'
--- partmap/gpt.c 2009-08-24 13:34:03 +0000
+++ partmap/gpt.c 2009-12-29 09:04:06 +0000
@@ -162,14 +162,7 @@
static char *
gpt_partition_map_get_name (const grub_partition_t p)
{
- char *name;
-
- name = grub_malloc (13);
- if (! name)
- return 0;
-
- grub_sprintf (name, "%d", p->index + 1);
- return name;
+ return grub_asprintf ("%d", p->index + 1);
}
\f
=== modified file 'partmap/msdos.c'
--- partmap/msdos.c 2009-09-16 19:23:33 +0000
+++ partmap/msdos.c 2009-12-29 09:04:06 +0000
@@ -300,21 +300,15 @@
static char *
pc_partition_map_get_name (const grub_partition_t p)
{
- char *name;
struct grub_msdos_partition *pcdata = p->data;
- name = grub_malloc (13);
- if (! name)
- return 0;
-
if (pcdata->bsd_part < 0)
- grub_sprintf (name, "%d", pcdata->dos_part + 1);
+ return grub_asprintf ("%d", pcdata->dos_part + 1);
else if (pcdata->dos_part < 0)
- grub_sprintf (name, "%c", pcdata->bsd_part + 'a');
+ return grub_asprintf ("%c", pcdata->bsd_part + 'a');
else
- grub_sprintf (name, "%d,%c", pcdata->dos_part + 1, pcdata->bsd_part + 'a');
-
- return name;
+ return grub_asprintf ("%d,%c", pcdata->dos_part + 1,
+ pcdata->bsd_part + 'a');
}
\f
=== modified file 'partmap/sun.c'
--- partmap/sun.c 2009-08-24 13:34:03 +0000
+++ partmap/sun.c 2009-12-29 09:04:06 +0000
@@ -184,13 +184,7 @@
static char *
sun_partition_map_get_name (const grub_partition_t p)
{
- char *name;
-
- name = grub_malloc (13);
- if (name)
- grub_sprintf (name, "%d", p->index + 1);
-
- return name;
+ return grub_asprintf ("%d", p->index + 1);
}
/* Partition map type. */
=== modified file 'script/execute.c'
--- script/execute.c 2009-12-13 12:35:20 +0000
+++ script/execute.c 2009-12-29 09:04:06 +0000
@@ -92,7 +92,7 @@
grub_err_t ret = 0;
int argcount = 0;
grub_script_function_t func = 0;
- char errnobuf[6];
+ char errnobuf[18];
char *cmdname;
/* Lookup the command. */
@@ -123,7 +123,7 @@
}
grub_free (assign);
- grub_sprintf (errnobuf, "%d", grub_errno);
+ grub_snprintf (errnobuf, sizeof (errnobuf), "%d", grub_errno);
grub_env_set ("?", errnobuf);
return 0;
@@ -156,7 +156,7 @@
grub_free (args[i]);
grub_free (args);
- grub_sprintf (errnobuf, "%d", ret);
+ grub_snprintf (errnobuf, sizeof (errnobuf), "%d", ret);
grub_env_set ("?", errnobuf);
return ret;
=== modified file 'term/gfxterm.c'
--- term/gfxterm.c 2009-12-26 10:01:33 +0000
+++ term/gfxterm.c 2009-12-29 09:04:06 +0000
@@ -272,9 +272,9 @@
err = grub_video_set_mode (DEFAULT_VIDEO_MODE, video_hook);
else
{
- tmp = grub_malloc (grub_strlen (modevar)
- + sizeof (DEFAULT_VIDEO_MODE) + 1);
- grub_sprintf (tmp, "%s;" DEFAULT_VIDEO_MODE, modevar);
+ tmp = grub_asprintf ("%s;" DEFAULT_VIDEO_MODE, modevar);
+ if (!tmp)
+ return grub_errno;
err = grub_video_set_mode (tmp, video_hook);
grub_free (tmp);
}
=== modified file 'term/ieee1275/ofconsole.c'
--- term/ieee1275/ofconsole.c 2009-12-24 22:53:05 +0000
+++ term/ieee1275/ofconsole.c 2009-12-29 09:04:06 +0000
@@ -104,7 +104,7 @@
static void
grub_ofconsole_setcolorstate (grub_term_color_state state)
{
- char setcol[20];
+ char *setcol;
int fg;
int bg;
@@ -123,8 +123,10 @@
return;
}
- grub_sprintf (setcol, "\e[3%dm\e[4%dm", fg, bg);
- grub_ofconsole_writeesc (setcol);
+ setcol = grub_asprintf ("\e[3%dm\e[4%dm", fg, bg);
+ if (setcol)
+ grub_ofconsole_writeesc (setcol);
+ grub_free (setcol);
}
static void
@@ -287,15 +289,16 @@
static void
grub_ofconsole_gotoxy (grub_uint8_t x, grub_uint8_t y)
{
- char s[11]; /* 5 + 3 + 3. */
-
if (! grub_ieee1275_test_flag (GRUB_IEEE1275_FLAG_NO_ANSI))
{
+ char *s;
grub_curr_x = x;
grub_curr_y = y;
- grub_sprintf (s, "\e[%d;%dH", y + 1, x + 1);
- grub_ofconsole_writeesc (s);
+ s = grub_asprintf ("\e[%d;%dH", y + 1, x + 1);
+ if (s)
+ grub_ofconsole_writeesc (s);
+ grub_free (s);
}
else
{
=== modified file 'term/tparm.c'
--- term/tparm.c 2009-10-12 21:53:15 +0000
+++ term/tparm.c 2009-12-29 09:04:06 +0000
@@ -167,7 +167,7 @@
get_space(s_len + 1);
- (void) grub_sprintf(out_buff + out_used, fmt, s);
+ (void) grub_snprintf(out_buff + out_used, s_len + 1, fmt, s);
out_used += grub_strlen(out_buff + out_used);
}
@@ -179,7 +179,7 @@
get_space((unsigned) len + 1);
- (void) grub_sprintf(out_buff + out_used, fmt, number);
+ (void) grub_snprintf(out_buff + out_used, len + 1, fmt, number);
out_used += grub_strlen(out_buff + out_used);
}
=== modified file 'util/grub-fstest.c'
--- util/grub-fstest.c 2009-12-10 13:39:54 +0000
+++ util/grub-fstest.c 2009-12-29 09:04:06 +0000
@@ -278,18 +278,26 @@
static void
fstest (char **images, int num_disks, int cmd, int n, char **args)
{
- char host_file[128];
- char loop_name[8];
- char *argv[3] = { "-p", loop_name, host_file};
+ char *host_file;
+ char *loop_name;
+ char *argv[3] = { "-p" };
int i;
for (i = 0; i < num_disks; i++)
{
- if (grub_strlen (images[i]) + 7 > sizeof (host_file))
- grub_util_error ("Pathname %s too long.", images[i]);
-
- grub_sprintf (loop_name, "loop%d", i);
- grub_sprintf (host_file, "(host)%s", images[i]);
+ loop_name = grub_asprintf ("loop%d", i);
+ host_file = grub_asprintf ("(host)%s", images[i]);
+
+ if (!loop_name || !host_file)
+ {
+ grub_free (loop_name);
+ grub_free (host_file);
+ grub_util_error (grub_errmsg);
+ return;
+ }
+
+ argv[1] = loop_name;
+ argv[2] = host_file;
if (execute_command ("loopback", 3, argv))
grub_util_error ("loopback command fails.");
@@ -328,9 +336,19 @@
for (i = 0; i < num_disks; i++)
{
- grub_sprintf (loop_name, "loop%d", i);
+ grub_free (loop_name);
+ loop_name = grub_asprintf ("loop%d", i);
+ if (!loop_name)
+ {
+ grub_free (host_file);
+ grub_util_error (grub_errmsg);
+ return;
+ }
execute_command ("loopback", 2, argv);
}
+
+ grub_free (loop_name);
+ grub_free (host_file);
}
static struct option options[] = {
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 293 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] switch from sprintf to asprintf and snprintf
2009-12-29 9:30 [PATCH] switch from sprintf to asprintf and snprintf Vladimir 'φ-coder/phcoder' Serbinenko
@ 2010-01-01 9:32 ` Colin Watson
2010-01-01 11:52 ` Robert Millan
2010-01-17 11:59 ` Vladimir 'φ-coder/phcoder' Serbinenko
2010-01-01 11:51 ` Robert Millan
1 sibling, 2 replies; 11+ messages in thread
From: Colin Watson @ 2010-01-01 9:32 UTC (permalink / raw)
To: The development of GNU GRUB
On Tue, Dec 29, 2009 at 10:30:12AM +0100, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
> +char *EXPORT_FUNC(grub_asprintf) (const char *fmt, ...)
> + __attribute__ ((format (printf, 1, 2)));
It's very confusing that you've made grub_asprintf have a dramatically
different interface from asprintf. Perhaps you could call this
grub_xasprintf instead? (Although I notice that it doesn't die when
malloc fails, but merely returns NULL.)
> +char *EXPORT_FUNC(grub_avsprintf) (const char *fmt, va_list args);
The conventional spelling is vasprintf, not avsprintf.
--
Colin Watson [cjwatson@ubuntu.com]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] switch from sprintf to asprintf and snprintf
2009-12-29 9:30 [PATCH] switch from sprintf to asprintf and snprintf Vladimir 'φ-coder/phcoder' Serbinenko
2010-01-01 9:32 ` Colin Watson
@ 2010-01-01 11:51 ` Robert Millan
2010-01-17 12:02 ` Vladimir 'φ-coder/phcoder' Serbinenko
1 sibling, 1 reply; 11+ messages in thread
From: Robert Millan @ 2010-01-01 11:51 UTC (permalink / raw)
To: The development of GNU GRUB
On Tue, Dec 29, 2009 at 10:30:12AM +0100, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
> sprintf is potentially dangerous especially with gettext, when messages
> may be larger than coder would expect. I attach the patch to fix it
Could you split the patches into one for asprintf and one for *nprintf? The
asprintf one is something I'd really like to see in trunk. For the rest I'm
not so sure.
--
Robert Millan
"Be the change you want to see in the world" -- Gandhi
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] switch from sprintf to asprintf and snprintf
2010-01-01 9:32 ` Colin Watson
@ 2010-01-01 11:52 ` Robert Millan
2010-01-01 12:01 ` Colin Watson
2010-01-17 12:02 ` Vladimir 'φ-coder/phcoder' Serbinenko
2010-01-17 11:59 ` Vladimir 'φ-coder/phcoder' Serbinenko
1 sibling, 2 replies; 11+ messages in thread
From: Robert Millan @ 2010-01-01 11:52 UTC (permalink / raw)
To: The development of GNU GRUB
On Fri, Jan 01, 2010 at 09:32:24AM +0000, Colin Watson wrote:
> On Tue, Dec 29, 2009 at 10:30:12AM +0100, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
> > +char *EXPORT_FUNC(grub_asprintf) (const char *fmt, ...)
> > + __attribute__ ((format (printf, 1, 2)));
>
> It's very confusing that you've made grub_asprintf have a dramatically
> different interface from asprintf. Perhaps you could call this
> grub_xasprintf instead?
Is it feasible to implement the same interface as asprintf instead?
--
Robert Millan
"Be the change you want to see in the world" -- Gandhi
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] switch from sprintf to asprintf and snprintf
2010-01-01 11:52 ` Robert Millan
@ 2010-01-01 12:01 ` Colin Watson
2010-01-17 12:02 ` Vladimir 'φ-coder/phcoder' Serbinenko
1 sibling, 0 replies; 11+ messages in thread
From: Colin Watson @ 2010-01-01 12:01 UTC (permalink / raw)
To: The development of GNU GRUB
On Fri, Jan 01, 2010 at 12:52:56PM +0100, Robert Millan wrote:
> On Fri, Jan 01, 2010 at 09:32:24AM +0000, Colin Watson wrote:
> > On Tue, Dec 29, 2009 at 10:30:12AM +0100, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
> > > +char *EXPORT_FUNC(grub_asprintf) (const char *fmt, ...)
> > > + __attribute__ ((format (printf, 1, 2)));
> >
> > It's very confusing that you've made grub_asprintf have a dramatically
> > different interface from asprintf. Perhaps you could call this
> > grub_xasprintf instead?
>
> Is it feasible to implement the same interface as asprintf instead?
asprintf is not really the ideal interface; most people forget to handle
errors when using it, and it's verbose even when used well. If there's a
single approach to out-of-memory errors that's appropriate throughout,
then something like xasprintf is much better.
--
Colin Watson [cjwatson@ubuntu.com]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] switch from sprintf to asprintf and snprintf
2010-01-01 9:32 ` Colin Watson
2010-01-01 11:52 ` Robert Millan
@ 2010-01-17 11:59 ` Vladimir 'φ-coder/phcoder' Serbinenko
1 sibling, 0 replies; 11+ messages in thread
From: Vladimir 'φ-coder/phcoder' Serbinenko @ 2010-01-17 11:59 UTC (permalink / raw)
To: The development of GNU GRUB
[-- Attachment #1: Type: text/plain, Size: 593 bytes --]
Colin Watson wrote:
> On Tue, Dec 29, 2009 at 10:30:12AM +0100, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
>
>> +char *EXPORT_FUNC(grub_asprintf) (const char *fmt, ...)
>> + __attribute__ ((format (printf, 1, 2)));
>>
>
> It's very confusing that you've made grub_asprintf have a dramatically
> different interface from asprintf. Perhaps you could call this
> grub_xasprintf instead? (Although I notice that it doesn't die when
> malloc fails, but merely returns NULL.)
>
What about grub_aprintf ?
--
Regards
Vladimir 'φ-coder/phcoder' Serbinenko
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 293 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] switch from sprintf to asprintf and snprintf
2010-01-01 11:51 ` Robert Millan
@ 2010-01-17 12:02 ` Vladimir 'φ-coder/phcoder' Serbinenko
0 siblings, 0 replies; 11+ messages in thread
From: Vladimir 'φ-coder/phcoder' Serbinenko @ 2010-01-17 12:02 UTC (permalink / raw)
To: The development of GNU GRUB
[-- Attachment #1: Type: text/plain, Size: 809 bytes --]
Robert Millan wrote:
> On Tue, Dec 29, 2009 at 10:30:12AM +0100, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
>
>> sprintf is potentially dangerous especially with gettext, when messages
>> may be larger than coder would expect. I attach the patch to fix it
>>
>
> Could you split the patches into one for asprintf and one for *nprintf? The
> asprintf one is something I'd really like to see in trunk. For the rest I'm
> not so sure.
>
Implementing sprintf on top of this would be just a waste. It would need
to add conditions to ignore size limits. IMHO unavailability of sprintf
decreases temptaion of making a 1024-bytes buffer and hoping it fits, or
at very least we don't have a memory corruption when it doesn't
--
Regards
Vladimir 'φ-coder/phcoder' Serbinenko
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 293 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] switch from sprintf to asprintf and snprintf
2010-01-01 11:52 ` Robert Millan
2010-01-01 12:01 ` Colin Watson
@ 2010-01-17 12:02 ` Vladimir 'φ-coder/phcoder' Serbinenko
2010-01-19 22:29 ` Robert Millan
1 sibling, 1 reply; 11+ messages in thread
From: Vladimir 'φ-coder/phcoder' Serbinenko @ 2010-01-17 12:02 UTC (permalink / raw)
To: The development of GNU GRUB
[-- Attachment #1: Type: text/plain, Size: 731 bytes --]
Robert Millan wrote:
> On Fri, Jan 01, 2010 at 09:32:24AM +0000, Colin Watson wrote:
>
>> On Tue, Dec 29, 2009 at 10:30:12AM +0100, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
>>
>>> +char *EXPORT_FUNC(grub_asprintf) (const char *fmt, ...)
>>> + __attribute__ ((format (printf, 1, 2)));
>>>
>> It's very confusing that you've made grub_asprintf have a dramatically
>> different interface from asprintf. Perhaps you could call this
>> grub_xasprintf instead?
>>
>
> Is it feasible to implement the same interface as asprintf instead?
>
>
it's feasible but the only advantage it gives is the return value nobody
uses anyway
--
Regards
Vladimir 'φ-coder/phcoder' Serbinenko
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 293 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] switch from sprintf to asprintf and snprintf
2010-01-17 12:02 ` Vladimir 'φ-coder/phcoder' Serbinenko
@ 2010-01-19 22:29 ` Robert Millan
2010-01-19 22:56 ` Colin Watson
0 siblings, 1 reply; 11+ messages in thread
From: Robert Millan @ 2010-01-19 22:29 UTC (permalink / raw)
To: The development of GNU GRUB
On Sun, Jan 17, 2010 at 01:02:55PM +0100, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
> Robert Millan wrote:
> > On Fri, Jan 01, 2010 at 09:32:24AM +0000, Colin Watson wrote:
> >
> >> On Tue, Dec 29, 2009 at 10:30:12AM +0100, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
> >>
> >>> +char *EXPORT_FUNC(grub_asprintf) (const char *fmt, ...)
> >>> + __attribute__ ((format (printf, 1, 2)));
> >>>
> >> It's very confusing that you've made grub_asprintf have a dramatically
> >> different interface from asprintf. Perhaps you could call this
> >> grub_xasprintf instead?
> >>
> >
> > Is it feasible to implement the same interface as asprintf instead?
> >
> >
> it's feasible but the only advantage it gives is the return value nobody
> uses anyway
What about consistency with what programmers expect?
If you don't want the return value, you can just discard it.
--
Robert Millan
"Be the change you want to see in the world" -- Gandhi
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] switch from sprintf to asprintf and snprintf
2010-01-19 22:29 ` Robert Millan
@ 2010-01-19 22:56 ` Colin Watson
2010-01-19 23:16 ` Robert Millan
0 siblings, 1 reply; 11+ messages in thread
From: Colin Watson @ 2010-01-19 22:56 UTC (permalink / raw)
To: The development of GNU GRUB
On Tue, Jan 19, 2010 at 11:29:14PM +0100, Robert Millan wrote:
> On Sun, Jan 17, 2010 at 01:02:55PM +0100, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
> > Robert Millan wrote:
> > > On Fri, Jan 01, 2010 at 09:32:24AM +0000, Colin Watson wrote:
> > >> On Tue, Dec 29, 2009 at 10:30:12AM +0100, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
> > >>> +char *EXPORT_FUNC(grub_asprintf) (const char *fmt, ...)
> > >>> + __attribute__ ((format (printf, 1, 2)));
> > >>
> > >> It's very confusing that you've made grub_asprintf have a dramatically
> > >> different interface from asprintf. Perhaps you could call this
> > >> grub_xasprintf instead?
> > >
> > > Is it feasible to implement the same interface as asprintf instead?
> >
> > it's feasible but the only advantage it gives is the return value nobody
> > uses anyway
>
> What about consistency with what programmers expect?
>
> If you don't want the return value, you can just discard it.
asprintf is not really an advantageous interface to emulate. It
requires tedious manual error handling and hardly anyone gets it right
(GRUB didn't get it right across the board, before I converted it to
xasprintf!), not to mention that error_code = function (&string, ...) is
unpleasant to start with. xasprintf is a much nicer interface; simply
returning the string is what most programmers *actually* expect unless
they've already bent their brains around asprintf.
The only variance from xasprintf seems to be what the error behaviour
ought to be.
--
Colin Watson [cjwatson@ubuntu.com]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] switch from sprintf to asprintf and snprintf
2010-01-19 22:56 ` Colin Watson
@ 2010-01-19 23:16 ` Robert Millan
0 siblings, 0 replies; 11+ messages in thread
From: Robert Millan @ 2010-01-19 23:16 UTC (permalink / raw)
To: The development of GNU GRUB
On Tue, Jan 19, 2010 at 10:56:48PM +0000, Colin Watson wrote:
> On Tue, Jan 19, 2010 at 11:29:14PM +0100, Robert Millan wrote:
> > On Sun, Jan 17, 2010 at 01:02:55PM +0100, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
> > > Robert Millan wrote:
> > > > On Fri, Jan 01, 2010 at 09:32:24AM +0000, Colin Watson wrote:
> > > >> On Tue, Dec 29, 2009 at 10:30:12AM +0100, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
> > > >>> +char *EXPORT_FUNC(grub_asprintf) (const char *fmt, ...)
> > > >>> + __attribute__ ((format (printf, 1, 2)));
> > > >>
> > > >> It's very confusing that you've made grub_asprintf have a dramatically
> > > >> different interface from asprintf. Perhaps you could call this
> > > >> grub_xasprintf instead?
> > > >
> > > > Is it feasible to implement the same interface as asprintf instead?
> > >
> > > it's feasible but the only advantage it gives is the return value nobody
> > > uses anyway
> >
> > What about consistency with what programmers expect?
> >
> > If you don't want the return value, you can just discard it.
>
> asprintf is not really an advantageous interface to emulate. It
> requires tedious manual error handling and hardly anyone gets it right
> (GRUB didn't get it right across the board, before I converted it to
> xasprintf!), not to mention that error_code = function (&string, ...) is
> unpleasant to start with. xasprintf is a much nicer interface; simply
> returning the string is what most programmers *actually* expect unless
> they've already bent their brains around asprintf.
Meh, I guess I'm one of those programmers who bent their brains.
Ok, feel free to go with proposed interface as grub_xasprintf().
--
Robert Millan
"Be the change you want to see in the world" -- Gandhi
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2010-01-19 23:16 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-12-29 9:30 [PATCH] switch from sprintf to asprintf and snprintf Vladimir 'φ-coder/phcoder' Serbinenko
2010-01-01 9:32 ` Colin Watson
2010-01-01 11:52 ` Robert Millan
2010-01-01 12:01 ` Colin Watson
2010-01-17 12:02 ` Vladimir 'φ-coder/phcoder' Serbinenko
2010-01-19 22:29 ` Robert Millan
2010-01-19 22:56 ` Colin Watson
2010-01-19 23:16 ` Robert Millan
2010-01-17 11:59 ` Vladimir 'φ-coder/phcoder' Serbinenko
2010-01-01 11:51 ` Robert Millan
2010-01-17 12:02 ` Vladimir 'φ-coder/phcoder' Serbinenko
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.