From: "Darrick J. Wong" <darrick.wong@oracle.com>
To: sandeen@sandeen.net, linux-xfs@vger.kernel.org
Subject: Re: [PATCH 1/2] xfs_db: btheight should check geometry more carefully
Date: Thu, 26 Sep 2019 10:38:43 -0700 [thread overview]
Message-ID: <20190926173843.GD9916@magnolia> (raw)
In-Reply-To: <20190926091147.dbjrf5i7rfgmzehb@pegasus.maiolino.io>
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@oracle.com>
> >
> > 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@oracle.com>
> > ---
> >
>
> 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
>
next prev parent reply other threads:[~2019-09-26 17:38 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-09-25 21:40 [PATCH 0/2] xfs_db: fixes to the btheight command Darrick J. Wong
2019-09-25 21:40 ` [PATCH 1/2] xfs_db: btheight should check geometry more carefully Darrick J. Wong
2019-09-26 9:11 ` Carlos Maiolino
2019-09-26 17:38 ` Darrick J. Wong [this message]
2019-09-27 12:30 ` Carlos Maiolino
2019-09-26 19:09 ` [PATCH v2 " Darrick J. Wong
2019-10-01 6:40 ` Carlos Maiolino
2019-09-30 7:55 ` [PATCH " Christoph Hellwig
2019-09-25 21:40 ` [PATCH 2/2] xfs_db: calculate iext tree geometry in btheight command Darrick J. Wong
2019-09-26 9:20 ` Carlos Maiolino
2019-09-26 17:40 ` Darrick J. Wong
2019-09-26 19:09 ` [PATCH v2 " Darrick J. Wong
2019-09-26 21:41 ` [PATCH " Dave Chinner
2019-09-27 2:58 ` Darrick J. Wong
2019-09-27 22:08 ` Dave Chinner
2019-09-30 7:58 ` Christoph Hellwig
2019-09-30 16:11 ` Darrick J. Wong
2019-10-01 6:24 ` Christoph Hellwig
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190926173843.GD9916@magnolia \
--to=darrick.wong@oracle.com \
--cc=linux-xfs@vger.kernel.org \
--cc=sandeen@sandeen.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).