Re: [PATCH v5 02/28] btrfs: Get zone information of zoned block devices

From: Naohiro Aota <naohiro.aota@wdc.com>
To: dsterba@suse.cz, Johannes Thumshirn <jthumshirn@suse.de>,
	linux-btrfs@vger.kernel.org, David Sterba <dsterba@suse.com>,
	Chris Mason <clm@fb.com>, Josef Bacik <josef@toxicpanda.com>,
	Nikolay Borisov <nborisov@suse.com>,
	Damien Le Moal <damien.lemoal@wdc.com>,
	Hannes Reinecke <hare@suse.com>,
	Anand Jain <anand.jain@oracle.com>,
	linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH v5 02/28] btrfs: Get zone information of zoned block devices
Date: Thu, 5 Dec 2019 15:29:19 +0900	[thread overview]
Message-ID: <20191205062919.mgpqe6gnbpahwaic@naota.dhcp.fujisawa.hgst.com> (raw)
In-Reply-To: <20191204172234.GI2734@suse.cz>

On Wed, Dec 04, 2019 at 06:22:34PM +0100, David Sterba wrote:
>On Wed, Dec 04, 2019 at 04:37:32PM +0100, Johannes Thumshirn wrote:
>> On Wed, Dec 04, 2019 at 05:17:09PM +0900, Naohiro Aota wrote:
>> [..]
>>
>> > +#define LEN (sizeof(device->fs_info->sb->s_id) + sizeof("(device )") - 1)
>> > +	char devstr[LEN];
>> > +	const int len = LEN;
>> > +#undef LEN
>>
>> Why not:
>> 	const int len = sizeof(device->fs_info->sb->s_id)
>> 					+ sizeof("(device )") - 1;
>> 	char devstr[len];
>
>len is used only once for snprintf to devstr, so there it can be
>replaced by sizeof(devstr) and the sizeof()+sizeof() can be used for
>devstr declaration.
>

That's better. I'll fix in that way.

>The size of devstr seems to be one byte shorter than needed:
>
>> > +		snprintf(devstr, len, " (device %s)",
>> > +			 device->fs_info->sb->s_id);
>
>There's a leading " " at the begining that I don't see accounted for.

Oops, I'll fix. Thanks.