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"),
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
