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) {

Reply via email to