On 01/20/2010 12:46 PM, Jim Meyering wrote:
Jim Meyering wrote:
This looks pretty clear. It calls closedir upon successful return, but not in the error case.
From 526a2bc2d87f0bf7a0c16a2a96316e5c1d5dae2e Mon Sep 17 00:00:00 2001 From: Jim Meyering<meyering@redhat.com> Date: Wed, 20 Jan 2010 17:49:35 +0100 Subject: [PATCH] node_device_linux_sysfs.c: avoid opendir/fd leak on error path
* src/node_device/node_device_linux_sysfs.c(get_virtual_functions_linux): Don't leak a DIR buffer and file descriptor on error path. --- src/node_device/node_device_linux_sysfs.c | 4 +++- 1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/src/node_device/node_device_linux_sysfs.c b/src/node_device/node_device_linux_sysfs.c index ff7aaf0..e611e1a 100644 --- a/src/node_device/node_device_linux_sysfs.c +++ b/src/node_device/node_device_linux_sysfs.c @@ -2,7 +2,7 @@ * node_device_hal_linuc.c: Linux specific code to gather device data * not available through HAL. * - * Copyright (C) 2009 Red Hat, Inc. + * Copyright (C) 2009-2010 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -371,6 +371,8 @@ int get_virtual_functions_linux(const char *sysfs_path, ret = 0;
out: + if (dir) + closedir(dir); VIR_FREE(device_link); return 0; }
That patch is incomplete. Would have been obvious if I'd included a few more lines of context above that "out:" label:
closedir(dir); ret = 0;
It lacked a last-minute addition that is required. I'll fold this in:
diff --git a/src/node_device/node_device_linux_sysfs.c b/src/node_device/node_device_linux_sysfs.c index e611e1a..674ee26 100644 --- a/src/node_device/node_device_linux_sysfs.c +++ b/src/node_device/node_device_linux_sysfs.c @@ -367,6 +367,7 @@ int get_virtual_functions_linux(const char *sysfs_path, }
closedir(dir); + dir = NULL;
ret = 0;
Here's the revised patch:
From a331c0194def0fe24f8b40d4ef91525eacff4cfb Mon Sep 17 00:00:00 2001 From: Jim Meyering<meyering@redhat.com> Date: Wed, 20 Jan 2010 17:49:35 +0100 Subject: [PATCH] node_device_linux_sysfs.c: avoid opendir/fd leak on error path
* src/node_device/node_device_linux_sysfs.c(get_virtual_functions_linux): Don't leak a DIR buffer and file descriptor on error path. --- src/node_device/node_device_linux_sysfs.c | 5 ++++- 1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/src/node_device/node_device_linux_sysfs.c b/src/node_device/node_device_linux_sysfs.c index ff7aaf0..674ee26 100644 --- a/src/node_device/node_device_linux_sysfs.c +++ b/src/node_device/node_device_linux_sysfs.c @@ -2,7 +2,7 @@ * node_device_hal_linuc.c: Linux specific code to gather device data * not available through HAL. * - * Copyright (C) 2009 Red Hat, Inc. + * Copyright (C) 2009-2010 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -367,10 +367,13 @@ int get_virtual_functions_linux(const char *sysfs_path, }
closedir(dir); + dir = NULL;
ret = 0;
out: + if (dir) + closedir(dir); VIR_FREE(device_link); return 0; } -- 1.6.6.516.gb72f
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Since dir is initialized to NULL, you could also simplify things by removing the closedir/dir = NULL statements above the out: label. Either way, this version looks safe to me. Dave