Skip to contentSkip to conversation

R: Re: [RFC PATCH V6 2/2] Btrfs: Add a new ioctl to change the label of a mounted file system

DISCUSSION [email protected] 1 participant 1 message Dec 19, 2012 Btrfs file system

lore
Share & tools
lore
#0 lore raw

Hi Jeff,

below my comments

Quoted 5 lines

----Messaggio originale----
Da: [email protected]
Data: 19/12/2012 4.42
A: <[email protected]>
Cc: "Goffredo Baroncelli"<[email protected]>, <[email protected]>, <linux-

[email protected]>, <[email protected]>

Quoted 1 line

Ogg: Re: [RFC PATCH V6 2/2] Btrfs: Add a new ioctl to change the label of a

mounted file system

Quoted 12 lines

Hi Goffredo,

Thanks for your review.

Quoted 6 lines · Goffredo Baroncelli

Hi Jeff,

On 12/18/2012 04:31 AM, Miao Xie wrote:
[...]

Quoted 1 line
Quoted 1 line
fs/btrfs/ioctl.c +0 -0

[...]

[...]

Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
fs/btrfs/ioctl.c +4 -0
+
+	if (strnlen(label, BTRFS_LABEL_SIZE) == BTRFS_LABEL_SIZE)
+		return -EINVAL;
+

[....]

Quoted 13 lines

Review

Quoted 6 lines
Quoted 2 lines

fs/btrfs/ioctl.c

Quoted 2 lines
fs/btrfs/ioctl.c +2 -0
+
+	strcpy(super_block->label, label);

I think that you removed for mistake the following line

(quoted diff) +1 -0
+	label[BTRFS_LABEL_SIZE - 1] = '\0';

I removed it since it was used to cut the label string off the max array size but now we have the previous strnlen().

Quoted 5 lines

In the V5 patch it was present.

May be we could replace strcpy() with strlcpy(super_block->label, label, BTRFS_LABEL_SIZE-1) ?

[...]

Quoted 2 lines

But does the current implementation make anything wrong? :)

Sorry, you are right. The check of strnlen() is enough...

Quoted 9 lines

Thanks,
-Jeff

Quoted 4 lines

BR
G.Baroncelli