Calls to fs_table_insert() are made in four places, and in all four the mount directory and device name arguments passed are the result of calls to strdup(). Rather than have all the callers handle allocating and freeing of these strings, consolidate that into fs_table_insert(). Only one place passes non-null values for the fslog and fsrt arguments, and in that case it's easier to keep the allocation of duplicate strings where they are in the caller. Add a comment in fs_table_insert() to ensure that's understood. Note also that fs_table_insert() is always called with both its dir and fsname arguments non-null, so drop a check for that at the top of the function. Signed-off-by: Alex Elder <aelder@xxxxxxx> --- libxcmd/paths.c | 138 +++++++++++++++++++++++++++---------------------------- 1 files changed, 67 insertions(+), 71 deletions(-) diff --git a/libxcmd/paths.c b/libxcmd/paths.c index 755307e..13873ef 100644 --- a/libxcmd/paths.c +++ b/libxcmd/paths.c @@ -95,21 +95,37 @@ fs_table_insert( { dev_t datadev, logdev, rtdev; struct fs_path *tmp_fs_table; - - if (!dir || !fsname) - return EINVAL; + int error; datadev = logdev = rtdev = 0; - if (fs_device_number(dir, &datadev)) - return errno; - if (fslog && fs_device_number(fslog, &logdev)) - return errno; - if (fsrt && fs_device_number(fsrt, &rtdev)) - return errno; + error = fs_device_number(dir, &datadev); + if (error) + goto out_nodev; + if (fslog && (error = fs_device_number(fslog, &logdev))) + goto out_nodev; + if (fsrt && (error = fs_device_number(fsrt, &rtdev))) + goto out_nodev; + + /* + * Make copies of the directory and data device path. + * The log device and real-time device, if non-null, + * are already the result of strdup() calls so we + * don't need to duplicate those. In fact, this + * function is assumed to "consume" both of those + * pointers, meaning if an error occurs they will + * both get freed. + */ + error = ENOMEM; + dir = strdup(dir); + if (!dir) + goto out_nodev; + fsname = strdup(fsname); + if (!fsname) + goto out_noname; tmp_fs_table = realloc(fs_table, sizeof(fs_path_t) * (fs_count + 1)); if (!tmp_fs_table) - return ENOMEM; + goto out_norealloc; fs_table = tmp_fs_table; fs_path = &fs_table[fs_count]; @@ -123,7 +139,19 @@ fs_table_insert( fs_path->fs_logdev = logdev; fs_path->fs_rtdev = rtdev; fs_count++; + return 0; + +out_norealloc: + free(fsname); +out_noname: + free(dir); +out_nodev: + /* "Consume" fslog and fsrt even if there's an error */ + free(fslog); + free(fsrt); + + return error; } void @@ -200,6 +228,14 @@ fs_cursor_next_entry( #if defined(HAVE_GETMNTENT) #include <mntent.h> +/* + * Determines whether the "logdev" or "rtdev" mount options are + * present for the given mount point. If so, the value for each (a + * device path) is returned in the pointers whose addresses are + * provided. The pointers are assigned NULL for an option not + * present. Note that the path buffers returned are allocated + * dynamically and it is the caller's responsibility to free them. + */ static void fs_extract_mount_options( struct mntent *mnt, @@ -243,11 +279,11 @@ fs_table_initialise_mounts( { struct mntent *mnt; FILE *mtp; - char *dir, *fsname, *fslog, *fsrt; + char *fslog, *fsrt; int error, found; error = found = 0; - dir = fsname = fslog = fsrt = NULL; + fslog = fsrt = NULL; if (!mtab_file) { mtab_file = PROC_MOUNTS; @@ -266,26 +302,16 @@ fs_table_initialise_mounts( (strcmp(path, mnt->mnt_fsname) != 0))) continue; found = 1; - dir = strdup(mnt->mnt_dir); - fsname = strdup(mnt->mnt_fsname); - if (!dir || !fsname) { - error = ENOMEM; - break; - } fs_extract_mount_options(mnt, &fslog, &fsrt); - if ((error = fs_table_insert(dir, 0, FS_MOUNT_POINT, - fsname, fslog, fsrt))) + error = fs_table_insert(mnt->mnt_dir, 0, FS_MOUNT_POINT, + mnt->mnt_fsname, fslog, fsrt); + if (error) break; } endmntent(mtp); if (!error && path && !found) error = ENXIO; - if (error) { - if (dir) free(dir); - if (fsrt) free(fsrt); - if (fslog) free(fslog); - if (fsname) free(fsname); - } + return error; } @@ -297,12 +323,9 @@ fs_table_initialise_mounts( char *path) { struct statfs *stats; - char *dir, *fsname, *fslog, *fsrt; int i, count, error, found; error = found = 0; - dir = fsname = fslog = fsrt = NULL; - if ((count = getmntinfo(&stats, 0)) < 0) { fprintf(stderr, _("%s: getmntinfo() failed: %s\n"), progname, strerror(errno)); @@ -317,25 +340,16 @@ fs_table_initialise_mounts( (strcmp(path, stats[i].f_mntfromname) != 0))) continue; found = 1; - dir = strdup(stats[i].f_mntonname); - fsname = strdup(stats[i].f_mntfromname); - if (!dir || !fsname) { - error = ENOMEM; - break; - } /* TODO: external log and realtime device? */ - if ((error = fs_table_insert(dir, 0, FS_MOUNT_POINT, - fsname, fslog, fsrt))) + error = fs_table_insert(stats[i].f_mntonname, 0, + FS_MOUNT_POINT, stats[i].f_mntfromname, + NULL, NULL); + if (error) break; } if (!error && path && !found) error = ENXIO; - if (error) { - if (dir) free(dir); - if (fsrt) free(fsrt); - if (fslog) free(fslog); - if (fsname) free(fsname); - } + return error; } @@ -387,7 +401,6 @@ fs_table_initialise_projects( fs_project_path_t *path; fs_path_t *fs; prid_t prid = 0; - char *dir = NULL, *fsname = NULL; int error = 0, found = 0; if (project) @@ -403,24 +416,17 @@ fs_table_initialise_projects( continue; } found = 1; - dir = strdup(path->pp_pathname); - fsname = strdup(fs->fs_name); - if (!dir || !fsname) { - error = ENOMEM; - break; - } - if ((error = fs_table_insert(dir, path->pp_prid, - FS_PROJECT_PATH, fsname, NULL, NULL))) + error = fs_table_insert(path->pp_pathname, path->pp_prid, + FS_PROJECT_PATH, fs->fs_name, + NULL, NULL); + if (error) break; } endprpathent(); if (!error && project && !found) error = ENOENT; - if (error) { - if (dir) free(dir); - if (fsname) free(fsname); - } + return error; } @@ -489,31 +495,21 @@ out_exit: void fs_table_insert_project_path( - char *udir, + char *dir, prid_t prid) { fs_path_t *fs; - char *dir = NULL, *fsname = NULL; int error = 0; - if ((fs = fs_mount_point_from_path(udir)) != NULL) { - dir = strdup(udir); - fsname = strdup(fs->fs_name); - if (dir && fsname) - error = fs_table_insert(dir, prid, - FS_PROJECT_PATH, fsname, NULL, NULL); - else - error = ENOMEM; + if ((fs = fs_mount_point_from_path(dir)) != NULL) { + error = fs_table_insert(dir, prid, FS_PROJECT_PATH, + fs->fs_name, NULL, NULL); } else error = ENOENT; if (error) { - if (dir) - free(dir); - if (fsname) - free(fsname); fprintf(stderr, _("%s: cannot setup path for project dir %s: %s\n"), - progname, udir, strerror(error)); + progname, dir, strerror(error)); exit(1); } } -- 1.7.6.2 _______________________________________________ xfs mailing list xfs@xxxxxxxxxxx http://oss.sgi.com/mailman/listinfo/xfs