On 14/07/2013 21:58, Wang Shilong wrote:
> Hello Anand and David,
>
> I use the tool valgrind to whether there exists memory leak:
>
> valgrind --tool=memcheck --trace-origins=yes --leak-chek=full btrfs sub list /mnt
> There still exists memory leak related to open_file_or_dir() (After applying this patch).
>
> I guess it is because we should call closedir(dirstram) rather than close(fd) if opening a directory.
> Maybe we can make open_file_or_dir() something like:
> open_file_or_dir(char *name, DIR **dirstream)
>
> if we opened a directory, we close it by calling closeidr(dirstream).
> elsewise, close(fd) is ok!..
>
> I am wondering why close(fd) can not just finish this work……
now I get your broader point of view. For some reason it
gave me an impression you are trying to handle a proper
cleanup operation if when dirfd(dirstream) fails and below
patch would just address that.
Now when open_file_or_dir() is successful we still need a
proper closing code. May be its a good idea to write a new
patch to include that.. it touches a lot of places.
Thanks, Anand
> Thanks,
> Wang
>> From: Wang Shilong <wangsl.fnst@xxxxxxxxxxxxxx>
>>
>> After calling opendir() successfully, closedir() should be
>> also called to free memory. Otherwise, memory leak happens.
>>
>> Signed-off-by: Anand Jain <anand.jain@xxxxxxxxxx>
>> Signed-off-by: Wang Shilong <wangsl.fnst@xxxxxxxxxxxxxx>
>> Reviewed-by: Anand Jain <anand.jain@xxxxxxxxxx>
>> ---
>> utils.c | 6 ++++--
>> 1 files changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/utils.c b/utils.c
>> index d3bec9b..4b3c778 100644
>> --- a/utils.c
>> +++ b/utils.c
>> @@ -1503,7 +1503,7 @@ int open_file_or_dir(const char *fname)
>> {
>> int ret;
>> struct stat st;
>> - DIR *dirstream;
>> + DIR *dirstream = NULL;
>> int fd;
>>
>> ret = stat(fname, &st);
>> @@ -1520,7 +1520,9 @@ int open_file_or_dir(const char *fname)
>> fd = open(fname, O_RDWR);
>> }
>> if (fd < 0) {
>> - return -3;
>> + fd = -3;
>> + if (dirstream)
>> + closedir(dirstream);
>> }
>> return fd;
>> }
>> --
>> 1.7.7.6
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
>> the body of a message to majordomo@xxxxxxxxxxxxxxx
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
