Linux Btrfs filesystem development
 help / color / mirror / Atom feed
From: "kreijack@inwind.it" <kreijack@inwind.it>
To: <jeff.liu@oracle.com>
Cc: Goffredo Baroncelli <kreijack@gmail.com>, <miaox@cn.fujitsu.com>,
	<linux-btrfs@vger.kernel.org>, <anand.jain@oracle.com>
Subject: R: Re: [RFC PATCH V6 2/2] Btrfs: Add a new ioctl to change the label of a mounted file system
Date: Wed, 19 Dec 2012 08:45:04 +0100 (CET)	[thread overview]
Message-ID: <28252221.2330091355903104722.JavaMail.defaultUser@defaultHost> (raw)

Hi Jeff,

below my comments

>----Messaggio originale----
>Da: jeff.liu@oracle.com
>Data: 19/12/2012 4.42
>A: <kreijack@inwind.it>
>Cc: "Goffredo Baroncelli"<kreijack@gmail.com>, <miaox@cn.fujitsu.com>, <linux-
btrfs@vger.kernel.org>, <anand.jain@oracle.com>
>Ogg: Re: [RFC PATCH V6 2/2] Btrfs: Add a new ioctl to change the label of a 
mounted file system
>
>Hi Goffredo,
>
>Thanks for your review.
>
>On 12/19/2012 02:00 AM, Goffredo Baroncelli wrote:
>> Hi Jeff,
>> 
>> On 12/18/2012 04:31 AM, Miao Xie wrote:
>> [...]
>>>> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
>> [...]
[...]
>>>> +
>>>> +	if (strnlen(label, BTRFS_LABEL_SIZE) == BTRFS_LABEL_SIZE)
>>>> +		return -EINVAL;
>>>> +
[....]
>>>> +
>>>> +	strcpy(super_block->label, label);
>> 
>> I think that you removed for mistake the following line
>> 
>> +	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().
>> 
>> In the V5 patch it was present.
>> 
>> May be we could replace strcpy() with strlcpy(super_block->label, label,
>> BTRFS_LABEL_SIZE-1) ?
[...]
>
>But does the current implementation make anything wrong? :)

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

>
>Thanks,
>-Jeff
>> 
>> BR
>> G.Baroncelli
>> 
>
>



                 reply	other threads:[~2012-12-19  7:45 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=28252221.2330091355903104722.JavaMail.defaultUser@defaultHost \
    --to=kreijack@inwind.it \
    --cc=anand.jain@oracle.com \
    --cc=jeff.liu@oracle.com \
    --cc=kreijack@gmail.com \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=miaox@cn.fujitsu.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox