Hi, Looks good to me, feel free to commit this,
Steve. On Sat, 2009-02-07 at 04:01 +0000, Andrew Price wrote: > This patch removes the calls to 'die' from check_for_gfs2 and adds error > reporting: It now returns -1 with errno set. All callers of this > function are also updated to check for errors. > > Signed-off-by: Andrew Price <a...@andrewprice.me.uk> > --- > > Let me know if you spot any problems. Thanks. > > P.S. I have also set up private branch andyp_libgfs2 to put my > non-askant work in, for convenience. This patch is in that branch. > > gfs2/libgfs2/libgfs2.h | 2 +- > gfs2/libgfs2/misc.c | 25 ++++++++++++++----------- > gfs2/mkfs/main_grow.c | 10 +++++++++- > gfs2/mkfs/main_jadd.c | 9 ++++++++- > gfs2/quota/check.c | 18 ++++++++++++++++-- > gfs2/quota/main.c | 36 ++++++++++++++++++++++++++++++++---- > gfs2/tool/df.c | 9 ++++++++- > gfs2/tool/misc.c | 36 ++++++++++++++++++++++++++++++++---- > gfs2/tool/tune.c | 18 ++++++++++++++++-- > 9 files changed, 136 insertions(+), 27 deletions(-) > > diff --git a/gfs2/libgfs2/libgfs2.h b/gfs2/libgfs2/libgfs2.h > index 1c743d1..8ea23f5 100644 > --- a/gfs2/libgfs2/libgfs2.h > +++ b/gfs2/libgfs2/libgfs2.h > @@ -627,7 +627,7 @@ extern uint32_t compute_heightsize(struct gfs2_sbd *sdp, > uint64_t *heightsize, > extern void compute_constants(struct gfs2_sbd *sdp); > extern int find_gfs2_meta(struct gfs2_sbd *sdp); > extern int dir_exists(const char *dir); > -extern void check_for_gfs2(struct gfs2_sbd *sdp); > +extern int check_for_gfs2(struct gfs2_sbd *sdp); > extern void mount_gfs2_meta(struct gfs2_sbd *sdp); > extern void cleanup_metafs(struct gfs2_sbd *sdp); > extern char *get_list(void); > diff --git a/gfs2/libgfs2/misc.c b/gfs2/libgfs2/misc.c > index 2cd6e46..6cd9335 100644 > --- a/gfs2/libgfs2/misc.c > +++ b/gfs2/libgfs2/misc.c > @@ -94,9 +94,9 @@ void compute_constants(struct gfs2_sbd *sdp) > sdp->sd_inptrs); > } > > -void check_for_gfs2(struct gfs2_sbd *sdp) > +int check_for_gfs2(struct gfs2_sbd *sdp) > { > - FILE *fp = fopen("/proc/mounts", "r"); > + FILE *fp; > char buffer[PATH_MAX]; > char fstype[80]; > int fsdump, fspass, ret; > @@ -106,12 +106,12 @@ void check_for_gfs2(struct gfs2_sbd *sdp) > > realname = realpath(sdp->path_name, NULL); > if (!realname) { > - perror(sdp->path_name); > - return; > + return -1; > } > + fp = fopen("/proc/mounts", "r"); > if (fp == NULL) { > - perror("open: /proc/mounts"); > - exit(EXIT_FAILURE); > + free(realname); > + return -1; > } > while ((fgets(buffer, PATH_MAX - 1, fp)) != NULL) { > buffer[PATH_MAX - 1] = 0; > @@ -134,15 +134,18 @@ void check_for_gfs2(struct gfs2_sbd *sdp) > continue; > > fclose(fp); > - if (strncmp(sdp->device_name, "/dev/loop", 9) == 0) > - die("Cannot perform this operation on a loopback GFS2 > mount.\n"); > - > free(realname); > - return; > + if (strncmp(sdp->device_name, "/dev/loop", 9) == 0) { > + errno = EINVAL; > + return -1; > + } > + > + return 0; > } > free(realname); > fclose(fp); > - die("gfs2 Filesystem %s is not mounted.\n", sdp->path_name); > + errno = EINVAL; > + return -1; > } > > static void lock_for_admin(struct gfs2_sbd *sdp) > diff --git a/gfs2/mkfs/main_grow.c b/gfs2/mkfs/main_grow.c > index 5ae40e8..bb2a587 100644 > --- a/gfs2/mkfs/main_grow.c > +++ b/gfs2/mkfs/main_grow.c > @@ -265,7 +265,15 @@ main_grow(int argc, char *argv[]) > die("can't open root directory %s: %s\n", > sdp->path_name, strerror(errno)); > > - check_for_gfs2(sdp); > + if (check_for_gfs2(sdp)) { > + if (errno == EINVAL) > + fprintf(stderr, > + "Not a valid GFS2 mount point: %s\n", > + sdp->path_name); > + else > + fprintf(stderr, "%s\n", strerror(errno)); > + exit(-1); > + } > sdp->device_fd = open(sdp->device_name, > (test ? O_RDONLY : O_RDWR)); > if (sdp->device_fd < 0) > diff --git a/gfs2/mkfs/main_jadd.c b/gfs2/mkfs/main_jadd.c > index f48dcd5..8d96886 100644 > --- a/gfs2/mkfs/main_jadd.c > +++ b/gfs2/mkfs/main_jadd.c > @@ -496,7 +496,14 @@ main_jadd(int argc, char *argv[]) > die("can't open root directory %s: %s\n", > sdp->path_name, strerror(errno)); > > - check_for_gfs2(sdp); > + if (check_for_gfs2(sdp)) { > + if (errno == EINVAL) > + fprintf(stderr, "Not a valid GFS2 mount point: %s\n", > + sdp->path_name); > + else > + fprintf(stderr, "%s\n", strerror(errno)); > + exit(-1); > + } > > gather_info(sdp); > > diff --git a/gfs2/quota/check.c b/gfs2/quota/check.c > index da32742..fdb3b42 100644 > --- a/gfs2/quota/check.c > +++ b/gfs2/quota/check.c > @@ -179,7 +179,14 @@ read_quota_file(struct gfs2_sbd *sdp, commandline_t > *comline, > char quota_file[BUF_SIZE]; > > strcpy(sdp->path_name, comline->filesystem); > - check_for_gfs2(sdp); > + if (check_for_gfs2(sdp)) { > + if (errno == EINVAL) > + fprintf(stderr, "Not a valid GFS2 mount point: %s\n", > + sdp->path_name); > + else > + fprintf(stderr, "%s\n", strerror(errno)); > + exit(-1); > + } > read_superblock(&sdp->sd_sb, sdp); > mount_gfs2_meta(sdp); > > @@ -451,7 +458,14 @@ set_list(struct gfs2_sbd *sdp, commandline_t *comline, > int user, > char *fs; > > strcpy(sdp->path_name, comline->filesystem); > - check_for_gfs2(sdp); > + if (check_for_gfs2(sdp)) { > + if (errno == EINVAL) > + fprintf(stderr, "Not a valid GFS2 mount point: %s\n", > + sdp->path_name); > + else > + fprintf(stderr, "%s\n", strerror(errno)); > + exit(-1); > + } > read_superblock(&sdp->sd_sb, sdp); > mount_gfs2_meta(sdp); > > diff --git a/gfs2/quota/main.c b/gfs2/quota/main.c > index 66822fc..544c793 100644 > --- a/gfs2/quota/main.c > +++ b/gfs2/quota/main.c > @@ -506,7 +506,14 @@ do_reset(struct gfs2_sbd *sdp, commandline_t *comline) > return; > > strcpy(sdp->path_name, comline->filesystem); > - check_for_gfs2(sdp); > + if (check_for_gfs2(sdp)) { > + if (errno == EINVAL) > + fprintf(stderr, "Not a valid GFS2 mount point: %s\n", > + sdp->path_name); > + else > + fprintf(stderr, "%s\n", strerror(errno)); > + exit(-1); > + } > read_superblock(&sdp->sd_sb, sdp); > mount_gfs2_meta(sdp); > > @@ -563,7 +570,14 @@ do_list(struct gfs2_sbd *sdp, commandline_t *comline) > die("need a filesystem to work on\n"); > > strcpy(sdp->path_name, comline->filesystem); > - check_for_gfs2(sdp); > + if (check_for_gfs2(sdp)) { > + if (errno == EINVAL) > + fprintf(stderr, "Not a valid GFS2 mount point: %s\n", > + sdp->path_name); > + else > + fprintf(stderr, "%s\n", strerror(errno)); > + exit(-1); > + } > read_superblock(&sdp->sd_sb, sdp); > mount_gfs2_meta(sdp); > > @@ -650,7 +664,14 @@ do_get_one(struct gfs2_sbd *sdp, commandline_t *comline, > char *filesystem) > char quota_file[BUF_SIZE]; > > strcpy(sdp->path_name, filesystem); > - check_for_gfs2(sdp); > + if (check_for_gfs2(sdp)) { > + if (errno == EINVAL) > + fprintf(stderr, "Not a valid GFS2 mount point: %s\n", > + sdp->path_name); > + else > + fprintf(stderr, "%s\n", strerror(errno)); > + exit(-1); > + } > read_superblock(&sdp->sd_sb, sdp); > mount_gfs2_meta(sdp); > > @@ -811,7 +832,14 @@ do_set(struct gfs2_sbd *sdp, commandline_t *comline) > die("need a new value\n"); > > strcpy(sdp->path_name, comline->filesystem); > - check_for_gfs2(sdp); > + if (check_for_gfs2(sdp)) { > + if (errno == EINVAL) > + fprintf(stderr, "Not a valid GFS2 mount point: %s\n", > + sdp->path_name); > + else > + fprintf(stderr, "%s\n", strerror(errno)); > + exit(-1); > + } > read_superblock(&sdp->sd_sb, sdp); > mount_gfs2_meta(sdp); > > diff --git a/gfs2/tool/df.c b/gfs2/tool/df.c > index 8e5800e..a2ab0e9 100644 > --- a/gfs2/tool/df.c > +++ b/gfs2/tool/df.c > @@ -86,7 +86,14 @@ do_df_one(char *path) > > memset(&sbd, 0, sizeof(struct gfs2_sbd)); > sbd.path_name = path; > - check_for_gfs2(&sbd); > + if (check_for_gfs2(&sbd)) { > + if (errno == EINVAL) > + fprintf(stderr, "Not a valid GFS2 mount point: %s\n", > + sbd.path_name); > + else > + fprintf(stderr, "%s\n", strerror(errno)); > + exit(-1); > + } > fs = mp2fsname(sbd.path_name); > > sbd.device_fd = open(sbd.device_name, O_RDONLY); > diff --git a/gfs2/tool/misc.c b/gfs2/tool/misc.c > index c591430..44e5126 100644 > --- a/gfs2/tool/misc.c > +++ b/gfs2/tool/misc.c > @@ -249,7 +249,14 @@ print_args(int argc, char **argv) > die("Usage: gfs2_tool getargs <mountpoint>\n"); > > sbd.path_name = argv[optind]; > - check_for_gfs2(&sbd); > + if (check_for_gfs2(&sbd)) { > + if (errno == EINVAL) > + fprintf(stderr, "Not a valid GFS2 mount point: %s\n", > + sbd.path_name); > + else > + fprintf(stderr, "%s\n", strerror(errno)); > + exit(-1); > + } > fs = mp2fsname(argv[optind]); > > memset(path, 0, PATH_MAX); > @@ -300,7 +307,14 @@ print_journals(int argc, char **argv) > if (sbd.path_fd < 0) > die("can't open root directory %s: %s\n", > sbd.path_name, strerror(errno)); > - check_for_gfs2(&sbd); > + if (check_for_gfs2(&sbd)) { > + if (errno == EINVAL) > + fprintf(stderr, "Not a valid GFS2 mount point: %s\n", > + sbd.path_name); > + else > + fprintf(stderr, "%s\n", strerror(errno)); > + exit(-1); > + } > sbd.device_fd = open(sbd.device_name, O_RDONLY); > if (sbd.device_fd < 0) > die("can't open device %s: %s\n", > @@ -363,7 +377,14 @@ do_shrink(int argc, char **argv) > die("Usage: gfs2_tool shrink <mountpoint>\n"); > > sbd.path_name = argv[optind]; > - check_for_gfs2(&sbd); > + if (check_for_gfs2(&sbd)) { > + if (errno == EINVAL) > + fprintf(stderr, "Not a valid GFS2 mount point: %s\n", > + sbd.path_name); > + else > + fprintf(stderr, "%s\n", strerror(errno)); > + exit(-1); > + } > fs = mp2fsname(argv[optind]); > > set_sysfs(fs, "shrink", "1"); > @@ -386,7 +407,14 @@ do_withdraw(int argc, char **argv) > die("Usage: gfs2_tool withdraw <mountpoint>\n"); > > sbd.path_name = argv[optind]; > - check_for_gfs2(&sbd); > + if (check_for_gfs2(&sbd)) { > + if (errno == EINVAL) > + fprintf(stderr, "Not a valid GFS2 mount point: %s\n", > + sbd.path_name); > + else > + fprintf(stderr, "%s\n", strerror(errno)); > + exit(-1); > + } > name = mp2fsname(argv[optind]); > > set_sysfs(name, "withdraw", "1"); > diff --git a/gfs2/tool/tune.c b/gfs2/tool/tune.c > index c083c97..6a8dcda 100644 > --- a/gfs2/tool/tune.c > +++ b/gfs2/tool/tune.c > @@ -44,7 +44,14 @@ get_tune(int argc, char **argv) > die("Usage: gfs2_tool gettune <mountpoint>\n"); > > sbd.path_name = argv[optind]; > - check_for_gfs2(&sbd); > + if (check_for_gfs2(&sbd)) { > + if (errno == EINVAL) > + fprintf(stderr, "Not a valid GFS2 mount point: %s\n", > + sbd.path_name); > + else > + fprintf(stderr, "%s\n", strerror(errno)); > + exit(-1); > + } > fs = mp2fsname(argv[optind]); > memset(path, 0, PATH_MAX); > snprintf(path, PATH_MAX - 1, "%s/%s/tune", SYS_BASE, fs); > @@ -95,7 +102,14 @@ set_tune(int argc, char **argv) > die("Usage: gfs2_tool settune <mountpoint> <parameter> > <value>\n"); > value = argv[optind++]; > > - check_for_gfs2(&sbd); > + if (check_for_gfs2(&sbd)) { > + if (errno == EINVAL) > + fprintf(stderr, "Not a valid GFS2 mount point: %s\n", > + sbd.path_name); > + else > + fprintf(stderr, "%s\n", strerror(errno)); > + exit(-1); > + } > fs = mp2fsname(sbd.path_name); > > if (strcmp(param, "quota_scale") == 0) {