diff mbox

[11/14] block/vvfat: Plug memory leak in check_directory_consistency()

Message ID 1401125835-21765-12-git-send-email-armbru@redhat.com
State New
Headers show

Commit Message

Markus Armbruster May 26, 2014, 5:37 p.m. UTC
On error path.  Introduced in commit a046433a.  Spotted by Coverity.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 block/vvfat.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Benoît Canet May 27, 2014, 11 a.m. UTC | #1
The Monday 26 May 2014 à 19:37:12 (+0200), Markus Armbruster wrote :
> On error path.  Introduced in commit a046433a.  Spotted by Coverity.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  block/vvfat.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/block/vvfat.c b/block/vvfat.c
> index 6a0d246..2c82a5c 100644
> --- a/block/vvfat.c
> +++ b/block/vvfat.c
> @@ -1864,7 +1864,7 @@ static int check_directory_consistency(BDRVVVFATState *s,
>  
>  	if (s->used_clusters[cluster_num] & USED_ANY) {
>  	    fprintf(stderr, "cluster %d used more than once\n", (int)cluster_num);
> -	    return 0;
> +            goto fail;

Hmm your editor display tabs at 8 chars so you put 12 chars of alignement.
Anyway the patch seems correct.

Reviewed-by: Benoit Canet <benoit@irqsave.net>

>  	}
>  	s->used_clusters[cluster_num] = USED_DIRECTORY;
>  
> -- 
> 1.9.3
> 
>
Markus Armbruster May 27, 2014, 4:17 p.m. UTC | #2
Benoît Canet <benoit.canet@irqsave.net> writes:

> The Monday 26 May 2014 à 19:37:12 (+0200), Markus Armbruster wrote :
>> On error path.  Introduced in commit a046433a.  Spotted by Coverity.
>> 
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>  block/vvfat.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/block/vvfat.c b/block/vvfat.c
>> index 6a0d246..2c82a5c 100644
>> --- a/block/vvfat.c
>> +++ b/block/vvfat.c
>> @@ -1864,7 +1864,7 @@ static int check_directory_consistency(BDRVVVFATState *s,
>>  
>>  	if (s->used_clusters[cluster_num] & USED_ANY) {
>>  	    fprintf(stderr, "cluster %d used more than once\n", (int)cluster_num);
>> -	    return 0;
>> +            goto fail;
>
> Hmm your editor display tabs at 8 chars so you put 12 chars of alignement.

You can have tab stops wherever you want, as long as you want them every
eight characters.

The body of the if is indeed indented 12 characters.  I expand tabs when
I touch a line.  Can look odd in diffs.

> Anyway the patch seems correct.
>
> Reviewed-by: Benoit Canet <benoit@irqsave.net>

Thanks!

[...]
Kevin Wolf May 28, 2014, 9:06 a.m. UTC | #3
Am 26.05.2014 um 19:37 hat Markus Armbruster geschrieben:
> On error path.  Introduced in commit a046433a.  Spotted by Coverity.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>

Heh, that fail: label you're jumping to is well hidden in some random
error check in the middle of the function. Pure awesomeness, this code.

(Your fix looks correct, but I just had to comment on this...)

Kevin
Markus Armbruster May 28, 2014, 9:27 a.m. UTC | #4
Kevin Wolf <kwolf@redhat.com> writes:

> Am 26.05.2014 um 19:37 hat Markus Armbruster geschrieben:
>> On error path.  Introduced in commit a046433a.  Spotted by Coverity.
>> 
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>
> Heh, that fail: label you're jumping to is well hidden in some random
> error check in the middle of the function. Pure awesomeness, this code.
>
> (Your fix looks correct, but I just had to comment on this...)

I expect nothing less than pure awesomeness from vvfat!
diff mbox

Patch

diff --git a/block/vvfat.c b/block/vvfat.c
index 6a0d246..2c82a5c 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -1864,7 +1864,7 @@  static int check_directory_consistency(BDRVVVFATState *s,
 
 	if (s->used_clusters[cluster_num] & USED_ANY) {
 	    fprintf(stderr, "cluster %d used more than once\n", (int)cluster_num);
-	    return 0;
+            goto fail;
 	}
 	s->used_clusters[cluster_num] = USED_DIRECTORY;