On Thu, Sep 26, 2019 at 10:38:43AM -0700, Darrick J. Wong wrote:
> On Thu, Sep 26, 2019 at 11:11:48AM +0200, Carlos Maiolino wrote:
> > On Wed, Sep 25, 2019 at 02:40:53PM -0700, Darrick J. Wong wrote:
> > > From: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> > >
> > > The btheight command needs to check user-supplied geometry more
> > > carefully so that we don't hit floating point exceptions.
> > >
> > > Coverity-id: 1453661, 1453659
> > > Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> > > ---
> > >
> >
> > Patch looks good, but.
> >
> >
> > > + if (record_size > blocksize) {
> > > + fprintf(stderr,
> > > + _("%s: record size must be less than %u bytes.\n"),
> >
> > Couldn't this message maybe be better saying "less than current blocksize"?
> > Saying it is less than X bytes sounds kind of meaningless, requiring a
> > trip to the code to understand what exactly 'bytes' mean here.
> >
> > Maybe something like:
> >
> > _("%s: record size must be less than current block size (%u).\n"),
>
> I think I'll change that to 'selected' from 'current' since the caller
> can change the block size with -b, but otherwise I agree.
Sounds good o me too, thanks
>
> --D
>
> >
> > Same for the next two.
> >
> > > + tag, blocksize);
> > > + goto out;
> > > + }
> > > +
> > > + if (key_size > blocksize) {
> > > + fprintf(stderr,
> > > + _("%s: key size must be less than %u bytes.\n"),
> > > + tag, blocksize);
> > > + goto out;
> > > + }
> > > +
> > > + if (ptr_size > blocksize) {
> > > + fprintf(stderr,
> > > + _("%s: pointer size must be less than %u bytes.\n"),
> > > + tag, blocksize);
> > > + goto out;
> > > + }
> > > +
> > > p = strtok(NULL, ":");
> > > if (p) {
> > > fprintf(stderr,
> > > @@ -211,13 +244,24 @@ report(
> > > int ret;
> > >
> > > ret = construct_records_per_block(tag, blocksize, records_per_block);
> > > - if (ret) {
> > > - printf(_("%s: Unable to determine records per block.\n"),
> > > - tag);
> > > + if (ret)
> > > return;
> > > - }
> > >
> > > if (report_what & REPORT_MAX) {
> > > + if (records_per_block[0] < 2) {
> > > + fprintf(stderr,
> > > +_("%s: cannot calculate best case scenario due to leaf geometry underflow.\n"),
> > > + tag);
> > > + return;
> > > + }
> > > +
> > > + if (records_per_block[1] < 4) {
> > > + fprintf(stderr,
> > > +_("%s: cannot calculate best case scenario due to node geometry underflow.\n"),
> > > + tag);
> > > + return;
> > > + }
> > > +
> > > printf(
> > > _("%s: best case per %u-byte block: %u records (leaf) / %u keyptrs (node)\n"),
> > > tag, blocksize, records_per_block[0],
> > > @@ -230,6 +274,20 @@ _("%s: best case per %u-byte block: %u records (leaf) / %u keyptrs (node)\n"),
> > > records_per_block[0] /= 2;
> > > records_per_block[1] /= 2;
> > >
> > > + if (records_per_block[0] < 1) {
> > > + fprintf(stderr,
> > > +_("%s: cannot calculate worst case scenario due to leaf geometry underflow.\n"),
> > > + tag);
> > > + return;
> > > + }
> > > +
> > > + if (records_per_block[1] < 2) {
> > > + fprintf(stderr,
> > > +_("%s: cannot calculate worst case scenario due to node geometry underflow.\n"),
> > > + tag);
> > > + return;
> > > + }
> > > +
> > > printf(
> > > _("%s: worst case per %u-byte block: %u records (leaf) / %u keyptrs (node)\n"),
> > > tag, blocksize, records_per_block[0],
> > > @@ -284,8 +342,26 @@ btheight_f(
> > > }
> > > }
> > >
> > > - if (argc == optind || blocksize <= 0 || blocksize > INT_MAX ||
> > > - nr_records == 0) {
> > > + if (nr_records == 0) {
> > > + fprintf(stderr,
> > > +_("Number of records must be greater than zero.\n"));
> > > + return 0;
> > > + }
> > > +
> > > + if (blocksize > INT_MAX) {
> > > + fprintf(stderr,
> > > +_("The largest block size this command will consider is %u bytes.\n"),
> > > + INT_MAX);
> > > + return 0;
> > > + }
> > > +
> > > + if (blocksize < 128) {
> > > + fprintf(stderr,
> > > +_("The smallest block size this command will consider is 128 bytes.\n"));
> > > + return 0;
> > > + }
> > > +
> > > + if (argc == optind) {
> > > btheight_help();
> > > return 0;
> > > }
> > >
> >
> > --
> > Carlos
> >
--
Carlos
