linux-kernel - Re: [PATCH 1/3] fs/ntfs3: Refactoring of indx

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <075d601a-1d79-acbd-2d03-92a1a73cf9c7@paragon-software.com>
Date:   Mon, 30 May 2022 20:11:24 +0300
From:   Konstantin Komarov <almaz.alexandrovich@...agon-software.com>
To:     Joe Perches <joe@...ches.com>, <ntfs3@...ts.linux.dev>
CC:     <linux-kernel@...r.kernel.org>, <linux-fsdevel@...r.kernel.org>
Subject: Re: [PATCH 1/3] fs/ntfs3: Refactoring of indx_find function

Hello.

Thanks for your input.
It's nice to have a clear typical return value, so I've updated patch.


On 5/27/22 19:07, Joe Perches wrote:
> On Fri, 2022-05-27 at 17:21 +0300, Almaz Alexandrovich wrote:
>> This commit makes function a bit more readable
> 
> trivia:
> 
>> diff --git a/fs/ntfs3/index.c b/fs/ntfs3/index.c
> []
>> @@ -1042,19 +1042,16 @@ int indx_find(struct ntfs_index *indx, struct ntfs_inode *ni,
>>    {
>>    	int err;
>>    	struct NTFS_DE *e;
>> -	const struct INDEX_HDR *hdr;
>>    	struct indx_node *node;
>>    
>>    	if (!root)
>>    		root = indx_get_root(&ni->dir, ni, NULL, NULL);
>>    
>>    	if (!root) {
>> -		err = -EINVAL;
>> -		goto out;
>> +		/* Should not happed. */
>> +		return -EINVAL;
> 
> s/happed/happen/
> 
>>    	for (;;) {
>>    		node = NULL;
>>    		if (*diff >= 0 || !de_has_vcn_ex(e)) {
>>    			*entry = e;
>> -			goto out;
>> +			return 0;
>>    		}
> 
> might be nicer with a break; or a while like
> 
> 	while (*diff < 0 && de_has_vcn_ex(e)) {
> 		node = NULL;
> 
> 
>>    		/* Read next level. */
>>    		err = indx_read(indx, ni, de_get_vbn(e), &node);
>>    		if (err)
>> -			goto out;
>> +			return err;
>>    
>>    		/* Lookup entry that is <= to the search value. */
>>    		e = hdr_find_e(indx, &node->index->ihdr, key, key_len, ctx,
>>    			       diff);
>>    		if (!e) {
>> -			err = -EINVAL;
>>    			put_indx_node(node);
>> -			goto out;
>> +			return -EINVAL;
>>    		}
>>    
>>    		fnd_push(fnd, node, e);
>>    	}
>> -
>> -out:
>> -	return err;
> 
> and a return 0;
> 
> or
> 	*entry = e;
> 	return 0;
> 
> so it appears that the function has a typical return value.
>