svn commit: r329830 - head/sbin/nvmecontrol

[Chuck Tuffli]

On Thu, Feb 22, 2018 at 10:04 AM, Ian Lepore <ian at freebsd.org> wrote:
> On Thu, 2018-02-22 at 17:47 +0000, Alan Somers wrote:
>> Author: asomers
>> Date: Thu Feb 22 17:47:16 2018
>> New Revision: 329830
>> URL: https://svnweb.freebsd.org/changeset/base/329830
>>
>> Log:
>>   nvmecontrol: fix build on amd64/clang
>>
>>   Broken by:  329824
>>   Sponsored by:       Spectra Logic Corp
>>
>> Modified:
>>   head/sbin/nvmecontrol/identify.c
>>
>> Modified: head/sbin/nvmecontrol/identify.c
>> ==============================================================================
>> --- head/sbin/nvmecontrol/identify.c  Thu Feb 22 17:09:26 2018        (r329829)
>> +++ head/sbin/nvmecontrol/identify.c  Thu Feb 22 17:47:16 2018        (r329830)
>> @@ -112,7 +112,7 @@ print_controller(struct nvme_controller_data *cdata)
>>       if (cdata->mdts == 0)
>>               printf("Unlimited\n");
>>       else
>> -             printf("%ld\n", PAGE_SIZE * (1 << cdata->mdts));
>> +             printf("%d\n", PAGE_SIZE * (1 << cdata->mdts));
>>       printf("Controller ID:              0x%02x\n", cdata->ctrlr_id);
>>       printf("\n");
>
> If cdata->mdts > 19 that'll overflow 32 bits.  I'm not sure if that can
> happen in the real world or not.  If so, maybe it'd be better to do
>
>   printf("%ju\n", PAGE_SIZE * ((uintmax_t)1 << cdata->mdts));
>
> -- Ian

Yes, MDTS is an 8 bit value and thus can overflow in the way you
describe, but typically, it is in the range of 5-12. Also note that
multiplying by PAGE_SIZE isn't quite correct as MDTS is in units of
the NVMe Controller's advertised minimum page size (i.e. CAP.MPSMIN).
Most Controllers will default to a value of 4096 for MPSMIN, so
practically, this code will usually work.

--chuck