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 <[email protected]> > --- > 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);
