Hi, Ok, that looks good now,
Steve. On Thu, 2009-03-19 at 16:16 +0000, Andrew Price wrote: > This patch removes the calls to die from __get_sysfs and adds error > handling to it and its callers. To enable this, the prototype of > get_sysfs_uint had to be changed so that error status can be returned. > Callers to get_sysfs and get_sysfs_uint are also updated to handle error > cases where appropriate. > > Signed-off-by: Andrew Price <[email protected]> > --- > Thanks for the comments on the previous version, I'm glad one of us is wide > awake today :-) > > gfs2/libgfs2/libgfs2.h | 2 +- > gfs2/libgfs2/misc.c | 43 +++++++++++++++++++++++++++++++++---------- > gfs2/tool/df.c | 44 +++++++++++++++++++++++++++++++------------- > gfs2/tool/tune.c | 10 ++++++++-- > 4 files changed, 73 insertions(+), 26 deletions(-) > > diff --git a/gfs2/libgfs2/libgfs2.h b/gfs2/libgfs2/libgfs2.h > index 2e79ad4..d8faf45 100644 > --- a/gfs2/libgfs2/libgfs2.h > +++ b/gfs2/libgfs2/libgfs2.h > @@ -634,7 +634,7 @@ extern char *find_debugfs_mount(void); > extern char *mp2fsname(char *mp); > extern char *mp2fsname2(char *devname); > extern char *get_sysfs(char *fsname, char *filename); > -extern unsigned int get_sysfs_uint(char *fsname, char *filename); > +extern int get_sysfs_uint(char *fsname, char *filename, unsigned int *val); > extern int set_sysfs(char *fsname, char *filename, char *val); > extern int is_fsname(char *name); > > diff --git a/gfs2/libgfs2/misc.c b/gfs2/libgfs2/misc.c > index 320ce84..60b2d4f 100644 > --- a/gfs2/libgfs2/misc.c > +++ b/gfs2/libgfs2/misc.c > @@ -216,10 +216,12 @@ static char *__get_sysfs(char *fsname, char *filename) > > fd = open(path, O_RDONLY); > if (fd < 0) > - die("can't open %s: %s\n", path, strerror(errno)); > + return NULL; > rv = read(fd, sysfs_buf, PAGE_SIZE); > - if (rv < 0) > - die("can't read from %s: %s\n", path, strerror(errno)); > + if (rv < 0) { > + close(fd); > + return NULL; > + } > > close(fd); > return sysfs_buf; > @@ -227,17 +229,30 @@ static char *__get_sysfs(char *fsname, char *filename) > > char *get_sysfs(char *fsname, char *filename) > { > - char *p = strchr(__get_sysfs(fsname, filename), '\n'); > + char *s; > + char *p; > + > + s = __get_sysfs(fsname, filename); > + if (!s) > + return NULL; > + p = strchr(s, '\n'); > if (p) > *p = '\0'; > return sysfs_buf; > } > > -unsigned int get_sysfs_uint(char *fsname, char *filename) > +int get_sysfs_uint(char *fsname, char *filename, unsigned int *val) > { > - unsigned int x; > - sscanf(__get_sysfs(fsname, filename), "%u", &x); > - return x; > + char *s = __get_sysfs(fsname, filename); > + int ret; > + if (!s) > + return -1; > + ret = sscanf(s, "%u", val); > + if (1 != ret) { > + errno = ENOMSG; > + return -1; > + } > + return 0; > } > > int set_sysfs(char *fsname, char *filename, char *val) > @@ -313,6 +328,7 @@ mp2fsname2(char *mp) > char buffer[PATH_MAX], device_name[PATH_MAX]; > int fsdump, fspass, ret, found = 0; > char fspath[PATH_MAX], fsoptions[PATH_MAX], fstype[80]; > + char *id; > > /* Take care of trailing '/' */ > if (mp[strlen(mp) - 1] == '/') > @@ -361,7 +377,10 @@ mp2fsname2(char *mp) > if (de->d_name[0] == '.') > continue; > > - if (strcmp(get_sysfs(de->d_name, "id"), device_id) == 0) { > + id = get_sysfs(de->d_name, "id"); > + if (!id) > + continue; > + if (strcmp(id, device_id) == 0) { > fsname = strdup(de->d_name); > break; > } > @@ -399,6 +418,7 @@ char *mp2fsname(char *mp) > struct stat statbuf; > DIR *d; > struct dirent *de; > + char *id; > > if (stat(mp, &statbuf)) > return NULL; > @@ -415,7 +435,10 @@ char *mp2fsname(char *mp) > if (de->d_name[0] == '.') > continue; > > - if (strcmp(get_sysfs(de->d_name, "id"), device_id) == 0) { > + id = get_sysfs(de->d_name, "id"); > + if (!id) > + continue; > + if (strcmp(id, device_id) == 0) { > fsname = strdup(de->d_name); > break; > } > diff --git a/gfs2/tool/df.c b/gfs2/tool/df.c > index 7d2875d..c817813 100644 > --- a/gfs2/tool/df.c > +++ b/gfs2/tool/df.c > @@ -147,22 +147,40 @@ do_df_one(char *path) > printf(" Block size = %u\n", sbd.sd_sb.sb_bsize); > printf(" Journals = %u\n", journals); > printf(" Resource Groups = %"PRIu64"\n", rgrps); > - printf(" Mounted lock proto = \"%s\"\n", > - ((value = get_sysfs(fs, "args/lockproto"))[0]) > - ? value : sbd.sd_sb.sb_lockproto); > - printf(" Mounted lock table = \"%s\"\n", > - ((value = get_sysfs(fs, "args/locktable"))[0]) > - ? value : sbd.sd_sb.sb_locktable); > + value = get_sysfs(fs, "args/lockproto"); > + if (value) > + printf(" Mounted lock proto = \"%s\"\n", > + value[0] ? value : sbd.sd_sb.sb_lockproto); > + else > + printf(" Mounted lock proto = (Not found: %s)\n", > + strerror(errno)); > + > + value = get_sysfs(fs, "args/locktable"); > + if (value) > + printf(" Mounted lock table = \"%s\"\n", > + value[0] ? value : sbd.sd_sb.sb_locktable); > + else > + printf(" Mounted lock table = (Not found: %s)\n", > + strerror(errno)); > + > printf(" Mounted host data = \"%s\"\n", > get_sysfs(fs, "args/hostdata")); > printf(" Journal number = %s\n", get_sysfs(fs, "lockstruct/jid")); > - flags = get_sysfs_uint(fs, "lockstruct/flags"); > - printf(" Lock module flags = %x", flags); > - printf("\n"); > - printf(" Local flocks = %s\n", > - (get_sysfs_uint(fs, "args/localflocks")) ? "TRUE" : "FALSE"); > - printf(" Local caching = %s\n", > - (get_sysfs_uint(fs, "args/localcaching")) ? "TRUE" : "FALSE"); > + > + if (get_sysfs_uint(fs, "lockstruct/flags", &flags)) > + printf(" Lock module flags = (Not found: %s)\n", > strerror(errno)); > + else > + printf(" Lock module flags = %x\n", flags); > + > + if (get_sysfs_uint(fs, "args/localflocks", &flags)) > + printf(" Lock flocks = (Not found: %s)\n", strerror(errno)); > + else > + printf(" Local flocks = %s\n", flags ? "TRUE" : "FALSE"); > + > + if (get_sysfs_uint(fs, "args/localcaching", &flags)) > + printf(" Lock caching = (Not found: %s)\n", strerror(errno)); > + else > + printf(" Local caching = %s\n", flags ? "TRUE" : "FALSE"); > > /* Read the master statfs file */ > if (mount_gfs2_meta(&sbd)) { > diff --git a/gfs2/tool/tune.c b/gfs2/tool/tune.c > index b4b24d8..29c529b 100644 > --- a/gfs2/tool/tune.c > +++ b/gfs2/tool/tune.c > @@ -39,6 +39,7 @@ get_tune(int argc, char **argv) > double ratio; > unsigned int num, den; > struct gfs2_sbd sbd; > + char *value; > > if (optind == argc) > die("Usage: gfs2_tool gettune <mountpoint>\n"); > @@ -65,8 +66,13 @@ get_tune(int argc, char **argv) > continue; > snprintf(path, PATH_MAX - 1, "tune/%s", de->d_name); > if (strcmp(de->d_name, "quota_scale") == 0) { > - sscanf(get_sysfs(fs, "tune/quota_scale"), "%u %u", > - &num, &den); > + value = get_sysfs(fs, "tune/quota_scale"); > + if (!value) { > + printf("quota_scale = (Not found: %s)\n", > + strerror(errno)); > + continue; > + } > + sscanf(value, "%u %u", &num, &den); > ratio = (double)num / den; > printf("quota_scale = %.4f (%u, %u)\n", ratio, num, > den);
