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.
--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
>
