On 12/18/2012 10:21 AM, Miao Xie wrote:

On 	mon, 17 Dec 2012 18:34:41 +0100, Goffredo Baroncelli wrote:

quoted

On 12/17/2012 02:30 PM, Jeff Liu wrote:

quoted

On 12/17/2012 07:57 PM, Miao Xie wrote:

quoted

On 	mon, 17 Dec 2012 19:22:11 +0800, Jeff Liu wrote:

quoted

Introduce a new ioctl BTRFS_IOC_SET_FSLABEL to change the label of a mounted file system.

Signed-off-by: Jie Liu <redacted>
Signed-off-by: Anand Jain <redacted>
Cc: Miao Xie <redacted>

[...]

quoted

quoted

quoted

+
+	if (strlen(label) > BTRFS_LABEL_SIZE - 1)
+		return -EINVAL;

I think we should use strnlen()

AFAICS, strnlen() is better only if the caller need to get the length of
a length-limited string and make use of it proceeding, which means that
the procedure would not return an error even if the length is beyond the
limit.  Or if the caller need to examine if a length-limited string is
nul-terminated or not in a manner below,
if (strnlen(buf, MAX_BUF_SIZE) == MAX_BUF_SIZE) {
	....
}

I don't think it really needed here since the logic is clear with
strlen(), or Am I miss anything?

I think that Miao fears strlen() searching a zero could go beyond the
page limit touching an un-mapped page and raising an segmentation fault....

Yes, so I think the following check is better.

if (strnlen(buf, BTRFS_LABEL_SIZE) == BTRFS_LABEL_SIZE)
	return -EINVAL;

Generally speaking, the user would not input a large string for normal
purpose, so strnlen() will always have a bit waste(can be ignore here)
with the counter self-check. i.e. for (; count--, ;).

Thanks
Miao

quoted

I think that we should change the code as

+	label[BTRFS_LABEL_SIZE - 1] = 0;
+
+	if (strlen(label) > BTRFS_LABEL_SIZE - 1)
+		return -EINVAL;

Both suggestion are fine to me, but I prefer to above approach.

Thanks,
-Jeff