Initialize CLEANUP_* stack variables with NULL in various places.

Code like:

  CLEANUP_FREE char *buf;
  /* some code which might return early */
  buf = malloc (10);

is a potential bug because the free (*buf) might be called when buf is
an uninitialized pointer.  Initialize buf = NULL to avoid this.

Several of these are bugs, most are not bugs (because there is no
early return statement before the variable gets initialized).

However the compiler can elide the initialization, and even if it does
not the performance "penalty" is miniscule, and correctness is better.
This commit is contained in:
Richard W.M. Jones
2013-08-22 19:48:05 +01:00
parent fc2947b112
commit b1919066ca
14 changed files with 18 additions and 18 deletions

View File

@@ -688,7 +688,7 @@ do_output_blockdevs (void)
for (i = 0; devices[i] != NULL; ++i) {
int64_t size = -1;
CLEANUP_FREE_STRING_LIST char **parents = NULL;
CLEANUP_FREE char *dev;
CLEANUP_FREE char *dev = NULL;
dev = guestfs_canonical_device_name (g, devices[i]);
if (!dev)

View File

@@ -355,7 +355,7 @@ do_aug_ls (const char *path)
if (STREQ (path, "/"))
matches = do_aug_match ("/*");
else {
CLEANUP_FREE char *buf;
CLEANUP_FREE char *buf = NULL;
len += 3; /* / * + terminating \0 */
buf = malloc (len);

View File

@@ -325,7 +325,7 @@ debug_binaries (const char *subcmd, size_t argc, char *const *const argv)
{
int r;
char *out;
CLEANUP_FREE char *err;
CLEANUP_FREE char *err = NULL;
char cmd[256];
snprintf (cmd, sizeof (cmd),
@@ -395,7 +395,7 @@ debug_ls (const char *subcmd, size_t argc, char *const *const argv)
size_t i;
int r;
char *out;
CLEANUP_FREE char *err;
CLEANUP_FREE char *err = NULL;
cargv[0] = str_ls;
cargv[1] = "-a";
@@ -422,7 +422,7 @@ debug_ll (const char *subcmd, size_t argc, char *const *const argv)
size_t i;
int r;
char *out;
CLEANUP_FREE char *err;
CLEANUP_FREE char *err = NULL;
cargv[0] = str_ls;
cargv[1] = "-la";

View File

@@ -649,9 +649,9 @@ char *
do_get_e2attrs (const char *filename)
{
int r;
CLEANUP_FREE char *buf;
CLEANUP_FREE char *buf = NULL;
char *out;
CLEANUP_FREE char *err;
CLEANUP_FREE char *err = NULL;
size_t i, j;
buf = sysroot_path (filename);

View File

@@ -492,7 +492,7 @@ do_file (const char *path)
const char *flags = is_dev ? "-zbsL" : "-zb";
char *out;
CLEANUP_FREE char *err;
CLEANUP_FREE char *err = NULL;
int r = command (&out, &err, str_file, flags, path, NULL);
if (r == -1) {

View File

@@ -710,7 +710,7 @@ commandf (char **stdoutput, char **stderror, int flags, const char *name, ...)
{
va_list args;
/* NB: Mustn't free the strings which are on the stack. */
CLEANUP_FREE const char **argv;
CLEANUP_FREE const char **argv = NULL;
char *s;
size_t i;
int r;
@@ -754,7 +754,7 @@ int
commandrf (char **stdoutput, char **stderror, int flags, const char *name, ...)
{
va_list args;
CLEANUP_FREE const char **argv;
CLEANUP_FREE const char **argv = NULL;
char *s;
int i, r;

View File

@@ -86,7 +86,7 @@ char *
do_initrd_cat (const char *path, const char *filename, size_t *size_r)
{
char tmpdir[] = "/tmp/initrd-cat-XXXXXX";
CLEANUP_FREE char *cmd;
CLEANUP_FREE char *cmd = NULL;
struct stat statbuf;
int fd, r;
char *ret = NULL;

View File

@@ -131,7 +131,7 @@ do_llz (const char *path)
int r;
char *out;
CLEANUP_FREE char *err = NULL;
CLEANUP_FREE char *spath;
CLEANUP_FREE char *spath = NULL;
spath = sysroot_path (path);
if (!spath) {

View File

@@ -280,7 +280,7 @@ do_internal_lxattrlist (const char *path, char *const *names)
for (k = 0; names[k] != NULL; ++k) {
void *newptr;
CLEANUP_FREE char *pathname;
CLEANUP_FREE char *pathname = NULL;
/* Be careful in this loop about which errors cause the whole call
* to abort, and which errors allow us to continue processing

View File

@@ -217,7 +217,7 @@ run_copy_out (const char *cmd, size_t argc, char *argv[])
/* Download each remote one at a time using tar-out. */
int i, r;
for (i = 0; i < nr_remotes; ++i) {
CLEANUP_FREE char *remote;
CLEANUP_FREE char *remote = NULL;
/* Allow win:... prefix on remotes. */
remote = win_prefix (argv[i]);

View File

@@ -161,7 +161,7 @@ print_inspect_prompt (void)
{
size_t i;
CLEANUP_FREE char *name = NULL;
CLEANUP_FREE_STRING_LIST char **mountpoints;
CLEANUP_FREE_STRING_LIST char **mountpoints = NULL;
name = guestfs_inspect_get_product_name (g, root);
if (name && STRNEQ (name, "unknown"))

View File

@@ -892,7 +892,7 @@ check_fstab (guestfs_h *g, struct inspect_fs *fs)
CLEANUP_FREE_STRING_LIST char **entries = NULL;
char **entry;
char augpath[256];
CLEANUP_HASH_FREE Hash_table *md_map;
CLEANUP_HASH_FREE Hash_table *md_map = NULL;
/* Generate a map of MD device paths listed in /etc/mdadm.conf to MD device
* paths in the guestfs appliance */

View File

@@ -480,7 +480,7 @@ check_windows_system_registry (guestfs_h *g, struct inspect_fs *fs)
if (STRCASEEQLEN (key, "\\DosDevices\\", 12) &&
c_isalpha (key[12]) && key[13] == ':') {
/* Get the binary value. Is it a fixed disk? */
CLEANUP_FREE char *blob;
CLEANUP_FREE char *blob = NULL;
char *device;
size_t len;
int64_t type;

View File

@@ -1621,7 +1621,7 @@ make_drive_priv (guestfs_h *g, struct drive *drv,
drv_priv->format = drv->format ? safe_strdup (g, drv->format) : NULL;
}
else {
CLEANUP_FREE char *qemu_device;
CLEANUP_FREE char *qemu_device = NULL;
drv_priv->real_src.protocol = drive_protocol_file;
qemu_device = guestfs___drive_source_qemu_param (g, &drv->src);