[Ocfs2-devel] [PATCH] ocfs2: drop the unwanted null byte in dentry lock name

[Joseph Qi]

On 2013/11/28 7:38, Andrew Morton wrote:
> On Wed, 27 Nov 2013 17:05:00 +0800 Joseph Qi <joseph.qi at huawei.com> wrote:
> 
>> Dentry lock name is made up of the type 'N', the parent blkno and its
>> blkno represented as a binary value.
>> Now there is a null byte between parent blkno and the binary blkno
>> value, so drop it.
> 
> Geeze.
> 
> What are the runtime effects of this change?
> 

If the null byte exists, it is obviously we cannot print the full dentry
lock name using '%s'.
But the fact is, the blkno is stuffed with binary value, it may also
contain null bytes, so we still cannot get the full dentry lock name in
the message.
Maybe we should also distinguish dentry lock and other lockres when
print lock name. But this makes the code malformed.

>> --- a/fs/ocfs2/dlm/dlmdebug.c
>> +++ b/fs/ocfs2/dlm/dlmdebug.c
>> @@ -257,7 +257,7 @@ static int stringify_lockname(const char *lockname, int locklen, char *buf,
>>  #define OCFS2_DENTRY_LOCK_INO_START	18
>>  	if (*lockname == 'N') {
>>  		memcpy((__be64 *)&inode_blkno_be,
>> -		       (char *)&lockname[OCFS2_DENTRY_LOCK_INO_START],
>> +		       (char *)&lockname[OCFS2_DENTRY_LOCK_INO_START - 1],
> 
> And what's up with all the CrazyCasting?  &inode_blkno_be is *already*
> __be64* and the second arg to memcpy is `const void *', so the cast
> there is also unneeded.
> 
>>  		       sizeof(__be64));
>>  		out += snprintf(buf + out, len - out, "%.*s%08x",
>>  				OCFS2_DENTRY_LOCK_INO_START - 1, lockname,
>> diff --git a/fs/ocfs2/dlmglue.c b/fs/ocfs2/dlmglue.c
>> index 3a44a64..62f2387 100644
>> --- a/fs/ocfs2/dlmglue.c
>> +++ b/fs/ocfs2/dlmglue.c
>> @@ -586,7 +586,7 @@ static __u64 ocfs2_get_dentry_lock_ino(struct ocfs2_lock_res *lockres)
>>  {
>>  	__be64 inode_blkno_be;
>>  
>> -	memcpy(&inode_blkno_be, &lockres->l_name[OCFS2_DENTRY_LOCK_INO_START],
>> +	memcpy(&inode_blkno_be, &lockres->l_name[OCFS2_DENTRY_LOCK_INO_START - 1],
> 
> Like that.
> 
>>  	       sizeof(__be64));
>>  
>>  	return be64_to_cpu(inode_blkno_be);
>> @@ -631,7 +631,7 @@ void ocfs2_dentry_lock_res_init(struct ocfs2_dentry_lock *dl,
>>  
>>  	BUG_ON(len != (OCFS2_DENTRY_LOCK_INO_START - 1));
>>  
>> -	memcpy(&lockres->l_name[OCFS2_DENTRY_LOCK_INO_START], &inode_blkno_be,
>> +	memcpy(&lockres->l_name[OCFS2_DENTRY_LOCK_INO_START - 1], &inode_blkno_be,
>>  	       sizeof(__be64));
>>  
>>  	ocfs2_lock_res_init_common(OCFS2_SB(inode->i_sb), lockres,
>> -- 
>> 1.8.4.3
> 
> .
>