[Cluster-devel] [PATCH 1/2] libgfs2: Remove die from compute_heightsize
Steven Whitehouse
swhiteho at redhat.com
Tue Apr 7 12:32:06 UTC 2009
Hi,
Looks good to me,
Steve.
On Tue, 2009-04-07 at 13:25 +0100, Andrew Price wrote:
> This patch removes the call to die from compute_heightsize and adds
> error reporting. To do this the prototype had to be changed to return
> the max height value through a pointer so that the return value could
> indicate success or failure. The compute_constants prototype was updated
> similarly as it calls compute_heightsize. All calls to both functions
> are updated to check for errors and print the same error message as before.
>
> Signed-off-by: Andrew Price <andy at andrewprice.me.uk>
> ---
> gfs2/convert/gfs2_convert.c | 49 ++++++++++++++++++++++++++++---------------
> gfs2/edit/hexedit.c | 10 +++++++-
> gfs2/edit/savemeta.c | 5 +++-
> gfs2/fsck/initialize.c | 5 +++-
> gfs2/libgfs2/libgfs2.h | 6 ++--
> gfs2/libgfs2/misc.c | 40 ++++++++++++++++++----------------
> gfs2/mkfs/main_grow.c | 5 +++-
> gfs2/mkfs/main_jadd.c | 5 +++-
> gfs2/mkfs/main_mkfs.c | 5 +++-
> gfs2/tool/df.c | 5 +++-
> 10 files changed, 88 insertions(+), 47 deletions(-)
>
> diff --git a/gfs2/convert/gfs2_convert.c b/gfs2/convert/gfs2_convert.c
> index e34fd11..3b9da3c 100644
> --- a/gfs2/convert/gfs2_convert.c
> +++ b/gfs2/convert/gfs2_convert.c
> @@ -136,8 +136,8 @@ struct gfs1_jindex *sd_jindex = NULL; /* gfs1 journal index in memory */
> int gfs2_inptrs;
> uint64_t gfs2_heightsize[GFS2_MAX_META_HEIGHT];
> uint64_t gfs2_jheightsize[GFS2_MAX_META_HEIGHT];
> -int gfs2_max_height;
> -int gfs2_max_jheight;
> +uint32_t gfs2_max_height;
> +uint32_t gfs2_max_jheight;
>
> /* ------------------------------------------------------------------------- */
> /* This function is for libgfs's sake. */
> @@ -1090,7 +1090,10 @@ static int init(struct gfs2_sbd *sbp)
> osi_list_init(&sbp->rglist);
> init_buf_list(sbp, &sbp->buf_list, 128 << 20);
> init_buf_list(sbp, &sbp->nvbuf_list, 0xffffffff);
> - compute_constants(sbp);
> + if (compute_constants(sbp)) {
> + log_crit("Error: Bad constants (1)\n");
> + exit(-1);
> + }
>
> bh = bread(&sbp->buf_list, GFS2_SB_ADDR >> sbp->sd_fsb2bb_shift);
> memcpy(&raw_gfs1_ondisk_sb, (struct gfs1_sb *)bh->b_data,
> @@ -1103,26 +1106,34 @@ static int init(struct gfs2_sbd *sbp)
> sizeof(uint64_t);
> sbp->sd_jbsize = sbp->bsize - sizeof(struct gfs2_meta_header);
> brelse(bh, not_updated);
> - sbp->sd_max_height = compute_heightsize(sbp, sbp->sd_heightsize,
> - sbp->bsize, sbp->sd_diptrs,
> - sbp->sd_inptrs);
> - sbp->sd_max_jheight = compute_heightsize(sbp, sbp->sd_jheightsize,
> - sbp->sd_jbsize,
> - sbp->sd_diptrs,
> - sbp->sd_inptrs);
> + if (compute_heightsize(sbp, sbp->sd_heightsize, &sbp->sd_max_height,
> + sbp->bsize, sbp->sd_diptrs, sbp->sd_inptrs)) {
> + log_crit("Error: Bad constants (1)\n");
> + exit(-1);
> + }
> +
> + if (compute_heightsize(sbp, sbp->sd_jheightsize, &sbp->sd_max_jheight,
> + sbp->sd_jbsize, sbp->sd_diptrs, sbp->sd_inptrs)) {
> + log_crit("Error: Bad constants (1)\n");
> + exit(-1);
> + }
> /* -------------------------------------------------------- */
> /* Our constants are for gfs1. Need some for gfs2 as well. */
> /* -------------------------------------------------------- */
> gfs2_inptrs = (sbp->bsize - sizeof(struct gfs2_meta_header)) /
> sizeof(uint64_t); /* How many ptrs can we fit on a block? */
> memset(gfs2_heightsize, 0, sizeof(gfs2_heightsize));
> - gfs2_max_height = compute_heightsize(sbp, gfs2_heightsize,
> - sbp->bsize,
> - sbp->sd_diptrs, gfs2_inptrs);
> + if (compute_heightsize(sbp, gfs2_heightsize, &gfs2_max_height,
> + sbp->bsize, sbp->sd_diptrs, gfs2_inptrs)) {
> + log_crit("Error: Bad constants (1)\n");
> + exit(-1);
> + }
> memset(gfs2_jheightsize, 0, sizeof(gfs2_jheightsize));
> - gfs2_max_jheight = compute_heightsize(sbp, gfs2_jheightsize,
> - sbp->sd_jbsize,
> - sbp->sd_diptrs, gfs2_inptrs);
> + if (compute_heightsize(sbp, gfs2_jheightsize, &gfs2_max_jheight,
> + sbp->sd_jbsize, sbp->sd_diptrs, gfs2_inptrs)) {
> + log_crit("Error: Bad constants (1)\n");
> + exit(-1);
> + }
>
> /* ---------------------------------------------- */
> /* Make sure we're really gfs1 */
> @@ -1550,7 +1561,11 @@ int main(int argc, char **argv)
> /* ---------------------------------------------- */
> if (!error) {
> /* Now we've got to treat it as a gfs2 file system */
> - compute_constants(&sb2);
> + if (compute_constants(&sb2)) {
> + log_crit("Error: Bad constants (1)\n");
> + exit(-1);
> + }
> +
> /* Build the master subdirectory. */
> build_master(&sb2); /* Does not do inode_put */
> sb2.sd_sb.sb_master_dir = sb2.master_dir->i_di.di_num;
> diff --git a/gfs2/edit/hexedit.c b/gfs2/edit/hexedit.c
> index 8c57bee..ac21ebd 100644
> --- a/gfs2/edit/hexedit.c
> +++ b/gfs2/edit/hexedit.c
> @@ -1724,7 +1724,10 @@ void read_superblock(int fd)
> osi_list_init(&sbd.rglist);
> init_buf_list(&sbd, &sbd.buf_list, 128 << 20);
> init_buf_list(&sbd, &sbd.nvbuf_list, 0xffffffff);
> - compute_constants(&sbd);
> + if (compute_constants(&sbd)) {
> + fprintf(stderr, "Bad constants (1)\n");
> + exit(-1);
> + }
> gfs2_sb_in(&sbd.sd_sb, buf); /* parse it out into the sb structure */
> /* Check to see if this is really gfs1 */
> if (sbd1->sb_fs_format == GFS_FORMAT_FS &&
> @@ -1743,7 +1746,10 @@ void read_superblock(int fd)
> sbd.bsize = sbd.sd_sb.sb_bsize;
> if (!sbd.bsize)
> sbd.bsize = GFS2_DEFAULT_BSIZE;
> - compute_constants(&sbd);
> + if (compute_constants(&sbd)) {
> + fprintf(stderr, "Bad constants (1)\n");
> + exit(-1);
> + }
> block = 0x10 * (GFS2_DEFAULT_BSIZE / sbd.bsize);
> device_geometry(&sbd);
> fix_device_geometry(&sbd);
> diff --git a/gfs2/edit/savemeta.c b/gfs2/edit/savemeta.c
> index 096e3fc..7c2ba04 100644
> --- a/gfs2/edit/savemeta.c
> +++ b/gfs2/edit/savemeta.c
> @@ -483,7 +483,10 @@ void savemeta(char *out_fn, int saveoption)
> init_buf_list(&sbd, &sbd.nvbuf_list, 0xffffffff);
> if (!gfs1)
> sbd.sd_sb.sb_bsize = GFS2_DEFAULT_BSIZE;
> - compute_constants(&sbd);
> + if (compute_constants(&sbd)) {
> + fprintf(stderr, "Bad constants (1)\n");
> + exit(-1);
> + }
> if(gfs1) {
> sbd.bsize = sbd.sd_sb.sb_bsize;
> sbd.sd_inptrs = (sbd.bsize -
> diff --git a/gfs2/fsck/initialize.c b/gfs2/fsck/initialize.c
> index e06b4b6..9aaee47 100644
> --- a/gfs2/fsck/initialize.c
> +++ b/gfs2/fsck/initialize.c
> @@ -317,7 +317,10 @@ static int fill_super_block(struct gfs2_sbd *sdp)
> return -1;
> }
>
> - compute_constants(sdp);
> + if (compute_constants(sdp)) {
> + log_crit(_("Bad constants (1)\n"));
> + exit(-1);
> + }
> if(read_sb(sdp) < 0){
> return -1;
> }
> diff --git a/gfs2/libgfs2/libgfs2.h b/gfs2/libgfs2/libgfs2.h
> index 435617e..0742f9e 100644
> --- a/gfs2/libgfs2/libgfs2.h
> +++ b/gfs2/libgfs2/libgfs2.h
> @@ -626,9 +626,9 @@ extern int gfs2_query(int *setonabort, struct gfs2_options *opts,
>
> /* misc.c */
>
> -extern uint32_t compute_heightsize(struct gfs2_sbd *sdp, uint64_t *heightsize,
> - uint32_t bsize1, int diptrs, int inptrs);
> -extern void compute_constants(struct gfs2_sbd *sdp);
> +extern int compute_heightsize(struct gfs2_sbd *sdp, uint64_t *heightsize,
> + uint32_t *maxheight, uint32_t bsize1, int diptrs, int inptrs);
> +extern int compute_constants(struct gfs2_sbd *sdp);
> extern int find_gfs2_meta(struct gfs2_sbd *sdp);
> extern int dir_exists(const char *dir);
> extern int check_for_gfs2(struct gfs2_sbd *sdp);
> diff --git a/gfs2/libgfs2/misc.c b/gfs2/libgfs2/misc.c
> index 8441137..726c582 100644
> --- a/gfs2/libgfs2/misc.c
> +++ b/gfs2/libgfs2/misc.c
> @@ -24,31 +24,31 @@
>
> static char sysfs_buf[PAGE_SIZE];
>
> -uint32_t compute_heightsize(struct gfs2_sbd *sdp, uint64_t *heightsize,
> - uint32_t bsize1, int diptrs, int inptrs)
> +int compute_heightsize(struct gfs2_sbd *sdp, uint64_t *heightsize,
> + uint32_t *maxheight, uint32_t bsize1, int diptrs, int inptrs)
> {
> - int x;
> -
> heightsize[0] = sdp->bsize - sizeof(struct gfs2_dinode);
> heightsize[1] = bsize1 * diptrs;
> - for (x = 2;; x++) {
> + for (*maxheight = 2;; (*maxheight)++) {
> uint64_t space, d;
> uint32_t m;
>
> - space = heightsize[x - 1] * inptrs;
> + space = heightsize[*maxheight - 1] * inptrs;
> m = space % inptrs;
> d = space / inptrs;
>
> - if (d != heightsize[x - 1] || m)
> + if (d != heightsize[*maxheight - 1] || m)
> break;
> - heightsize[x] = space;
> + heightsize[*maxheight] = space;
> }
> - if (x > GFS2_MAX_META_HEIGHT)
> - die("bad constants (1)\n");
> - return x;
> + if (*maxheight > GFS2_MAX_META_HEIGHT) {
> + errno = EINVAL;
> + return -1;
> + }
> + return 0;
> }
>
> -void compute_constants(struct gfs2_sbd *sdp)
> +int compute_constants(struct gfs2_sbd *sdp)
> {
> uint32_t hash_blocks, ind_blocks, leaf_blocks;
> uint32_t tmp_blocks;
> @@ -85,13 +85,15 @@ void compute_constants(struct gfs2_sbd *sdp)
>
> sdp->sd_max_dirres = hash_blocks + ind_blocks + leaf_blocks;
>
> - sdp->sd_max_height = compute_heightsize(sdp, sdp->sd_heightsize,
> - sdp->bsize, sdp->sd_diptrs,
> - sdp->sd_inptrs);
> - sdp->sd_max_jheight = compute_heightsize(sdp, sdp->sd_jheightsize,
> - sdp->sd_jbsize,
> - sdp->sd_diptrs,
> - sdp->sd_inptrs);
> + if (compute_heightsize(sdp, sdp->sd_heightsize, &sdp->sd_max_height,
> + sdp->bsize, sdp->sd_diptrs, sdp->sd_inptrs)) {
> + return -1;
> + }
> + if (compute_heightsize(sdp, sdp->sd_jheightsize, &sdp->sd_max_jheight,
> + sdp->sd_jbsize, sdp->sd_diptrs, sdp->sd_inptrs)) {
> + return -1;
> + }
> + return 0;
> }
>
> int check_for_gfs2(struct gfs2_sbd *sdp)
> diff --git a/gfs2/mkfs/main_grow.c b/gfs2/mkfs/main_grow.c
> index e87623a..94c7d71 100644
> --- a/gfs2/mkfs/main_grow.c
> +++ b/gfs2/mkfs/main_grow.c
> @@ -292,7 +292,10 @@ main_grow(int argc, char *argv[])
>
> sdp->sd_sb.sb_bsize = GFS2_DEFAULT_BSIZE;
> sdp->bsize = sdp->sd_sb.sb_bsize;
> - compute_constants(sdp);
> + if (compute_constants(sdp)) {
> + log_crit(_("Bad constants (1)\n"));
> + exit(-1);
> + }
> if(read_sb(sdp) < 0)
> die( _("gfs: Error reading superblock.\n"));
>
> diff --git a/gfs2/mkfs/main_jadd.c b/gfs2/mkfs/main_jadd.c
> index 5f9e461..4779308 100644
> --- a/gfs2/mkfs/main_jadd.c
> +++ b/gfs2/mkfs/main_jadd.c
> @@ -506,7 +506,10 @@ void main_jadd(int argc, char *argv[])
> exit(-1);
> }
>
> - compute_constants(sdp);
> + if (compute_constants(sdp)) {
> + fprintf(stderr, _("Bad constants (1)\n"));
> + exit(-1);
> + }
> find_current_journals(sdp);
>
> total = sdp->orig_journals + sdp->md.journals;
> diff --git a/gfs2/mkfs/main_mkfs.c b/gfs2/mkfs/main_mkfs.c
> index edda1d0..ce3a6ae 100644
> --- a/gfs2/mkfs/main_mkfs.c
> +++ b/gfs2/mkfs/main_mkfs.c
> @@ -505,7 +505,10 @@ void main_mkfs(int argc, char *argv[])
> if (!sdp->override)
> are_you_sure(sdp);
>
> - compute_constants(sdp);
> + if (compute_constants(sdp)) {
> + fprintf(stderr, _("Bad constants (1)\n"));
> + exit(-1);
> + }
>
> /* Get the device geometry */
>
> diff --git a/gfs2/tool/df.c b/gfs2/tool/df.c
> index 53c645d..d444d3c 100644
> --- a/gfs2/tool/df.c
> +++ b/gfs2/tool/df.c
> @@ -130,7 +130,10 @@ do_df_one(char *path)
>
> gfs2_sb_in(&sbd.sd_sb, buf); /* parse it out into the sb structure */
> sbd.bsize = sbd.sd_sb.sb_bsize;
> - compute_constants(&sbd);
> + if (compute_constants(&sbd)) {
> + fprintf(stderr, _("Bad constants (1)\n"));
> + exit(-1);
> + }
>
> sbd.master_dir = gfs2_load_inode(&sbd,
> sbd.sd_sb.sb_master_dir.no_addr);
More information about the Cluster-devel
mailing list