[v2] mm: shmem: Fix variable type to int to evaluate negative conditions

[PATCH v2] mm: shmem: Fix variable type to int to evaluate negative conditions

Posted by Karan Sanghavi 1 year ago

Use signed data type for version handling
in shmem_parse_opt_casefold()

Fixes: 58e55efd6c72 ("tmpfs: Add casefold lookup support")
Reviewed-by: André Almeida <andrealmeid@igalia.com>
Reviewed-by: Gabriel Krisman Bertazi <gabriel@krisman.be>
Signed-off-by: Karan Sanghavi <karansanghvi98@gmail.com>
---
Coverity Message:
CID 1601569: (#1 of 1): Unsigned compared against 0 (NO_EFFECT)
unsigned_compare: This less-than-zero comparison of an unsigned 
value is never true. version < 0U

Coverity Link:
https://scan7.scan.coverity.com/#/project-view/10043/11354?selectedIssue=1601569
---
Changes in v2:
- Updated the log summary.
- Link to v1: https://lore.kernel.org/r/20241111-unsignedcompare1601569-v1-1-c4a9c3c75a52@gmail.com
---
 mm/shmem.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/shmem.c b/mm/shmem.c
index 7987deb2be9b..17a1d4fa8870 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -4367,7 +4367,7 @@ static int shmem_parse_opt_casefold(struct fs_context *fc, struct fs_parameter *
 				    bool latest_version)
 {
 	struct shmem_options *ctx = fc->fs_private;
-	unsigned int version = UTF8_LATEST;
+	int version = UTF8_LATEST;
 	struct unicode_map *encoding;
 	char *version_str = param->string + 5;
 

---
base-commit: 6d59cab07b8d74d0f0422b750038123334f6ecc2
change-id: 20241111-unsignedcompare1601569-05b5932145ef

Best regards,
-- 
Karan Sanghavi <karansanghvi98@gmail.com>

Re: [PATCH v2] mm: shmem: Fix variable type to int to evaluate negative conditions

Posted by Karan Sanghavi 11 months, 4 weeks ago

On Wed, Nov 27, 2024 at 06:07:25PM +0000, Karan Sanghavi wrote:
> Use signed data type for version handling
> in shmem_parse_opt_casefold()
> 
> Fixes: 58e55efd6c72 ("tmpfs: Add casefold lookup support")
> Reviewed-by: André Almeida <andrealmeid@igalia.com>
> Reviewed-by: Gabriel Krisman Bertazi <gabriel@krisman.be>
> Signed-off-by: Karan Sanghavi <karansanghvi98@gmail.com>
> ---
> Coverity Message:
> CID 1601569: (#1 of 1): Unsigned compared against 0 (NO_EFFECT)
> unsigned_compare: This less-than-zero comparison of an unsigned 
> value is never true. version < 0U
> 
> Coverity Link:
> https://scan7.scan.coverity.com/#/project-view/10043/11354?selectedIssue=1601569
> ---
> Changes in v2:
> - Updated the log summary.
> - Link to v1: https://lore.kernel.org/r/20241111-unsignedcompare1601569-v1-1-c4a9c3c75a52@gmail.com
> ---
>  mm/shmem.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mm/shmem.c b/mm/shmem.c
> index 7987deb2be9b..17a1d4fa8870 100644
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -4367,7 +4367,7 @@ static int shmem_parse_opt_casefold(struct fs_context *fc, struct fs_parameter *
>  				    bool latest_version)
>  {
>  	struct shmem_options *ctx = fc->fs_private;
> -	unsigned int version = UTF8_LATEST;
> +	int version = UTF8_LATEST;
>  	struct unicode_map *encoding;
>  	char *version_str = param->string + 5;
>  
> 
> ---
> base-commit: 6d59cab07b8d74d0f0422b750038123334f6ecc2
> change-id: 20241111-unsignedcompare1601569-05b5932145ef
> 
> Best regards,
> -- 
> Karan Sanghavi <karansanghvi98@gmail.com>
> 

Hello Markus,
I hope you are doing great,

Can you please let me know by can I expect this patch to be applied since I have changed the log
summary as you have mentioned in the last mail. 
Please do let me know if there anything else to be done for it to get
accepted 

Thank you,
Karan.

Re: [PATCH v2] mm: shmem: Fix variable type to int to evaluate negative conditions

Posted by Markus Elfring 11 months, 4 weeks ago

>> Use signed data type for version handling
>> in shmem_parse_opt_casefold()

How do you think about to move this information into the summary phrase?


…
>> ---
>> Coverity Message:
>> CID 1601569: (#1 of 1): Unsigned compared against 0 (NO_EFFECT)
>> unsigned_compare: This less-than-zero comparison of an unsigned
>> value is never true. version < 0U
…

May such information become a part for the final change description?

Regards,
Markus

Re: [PATCH v2] mm: shmem: Fix variable type to int to evaluate negative conditions

Posted by Karan Sanghavi 11 months, 3 weeks ago

On Mon, Dec 23, 2024 at 01:43:09PM +0100, Markus Elfring wrote:
> >> Use signed data type for version handling
> >> in shmem_parse_opt_casefold()
> 
> How do you think about to move this information into the summary phrase?
> 
>
should i add more info to this? as i am still not able to understand
what i need to do here. Can you please suggest what changes are you
suggesting here. 

> …
> >> ---
> >> Coverity Message:
> >> CID 1601569: (#1 of 1): Unsigned compared against 0 (NO_EFFECT)
> >> unsigned_compare: This less-than-zero comparison of an unsigned
> >> value is never true. version < 0U
> …
> 
> May such information become a part for the final change description?
>
ofcoure not, as per my understanding, text after '---' are not included
in the change descriptions. (Do correct me if i am wrong here) Thus no
need to worry for them getting added in the change log.

Thank you,
Karan.
> Regards,
> Markus

Re: [v2] mm: shmem: Fix variable type to int to evaluate negative conditions

Posted by Markus Elfring 11 months, 3 weeks ago

>>>> Use signed data type for version handling
>>>> in shmem_parse_opt_casefold()
>>
>> How do you think about to move this information into the summary phrase?
>>
> should i add more info to this? as i am still not able to understand
> what i need to do here. Can you please suggest what changes are you
> suggesting here.

I propose to refine the distribution of information between the patch subject
and the changelog.

Regards,
Markus