Matthew Daley
2013-Sep-18 03:37 UTC
[PATCH 00/28] Fixes for various minor Coverity issues, volume 2
Tested by building and creating a few PV and HVM domains. There
shouldn''t be
anything too complicated or controversial here.
Matthew Daley (28):
gnttab: remove unused shared header lookup
libxc: fix memory leak in load_p2m_frame_list error handling
libxc: move munmap into the loop it''s needed in in change_pte
libxl: fix libxl_string_list_length and its only caller
libxl: fix dispose without init of disk in cd_insert
libxl: fix leak of corename in handle_domain_death
libxl: fix leak of config_data in main_cpupoolcreate
libxl: fix leak of rune in main_remus
libxl: fix out-of-memory check in parse_global_config
libxl: only free cpupoolinfo if necessary in libxl_list_cpupool
libxl: only put poller if already gotten in libxl_event_wait
libxl: only free cputopology if it was allocated in
libxl__get_numa_candidate
libxl: only free cputopology if it was allocated in
libxl_{cpu,node}map_to_{node,cpu}map
libxl: only free console reader if it was allocated in main_dmesg
libxl: fix typo in libxl__hotplug_nic error checking
libxl: fix file open failure check in libxl__file_reference_map
libxl: gettimeofday doesn''t return an errno on failure
xenstored: handle unlikely failure better in ask_parents
xenstored: fix faulty check for bad handle in domain_init
xenstore: check socket path length before copying it
mini-os: fix nodename generation in init_netfront
mini-os: fix various memory leaks in blkfront
mini-os: fix various memory leaks in {fb,kbd}front
mini-os: fix various memory leaks in netfront
mini-os: fix various memory leaks in pcifront
mini-os: fix various memory leaks in consfront
mini-os: fix various memory leaks in various locations
gdbsx: clear sockaddr before using it
extras/mini-os/blkfront.c | 11 +++++----
extras/mini-os/console/xenbus.c | 9 +++----
extras/mini-os/fbfront.c | 48 ++++++++++++++++++++++++-------------
extras/mini-os/kernel.c | 7 ++++--
extras/mini-os/lib/xs.c | 2 ++
extras/mini-os/netfront.c | 23 +++++++++++-------
extras/mini-os/pcifront.c | 29 ++++++++++++++--------
tools/debugger/gdbsx/gx/gx_comm.c | 1 +
tools/libxc/xc_domain_restore.c | 1 +
tools/libxc/xc_offline_page.c | 6 ++---
tools/libxl/libxl.c | 5 ++--
tools/libxl/libxl_bootloader.c | 2 +-
tools/libxl/libxl_event.c | 2 +-
tools/libxl/libxl_internal.c | 2 +-
tools/libxl/libxl_linux.c | 2 +-
tools/libxl/libxl_numa.c | 3 ++-
tools/libxl/libxl_utils.c | 6 +++--
tools/libxl/xl.c | 4 ++--
tools/libxl/xl_cmdimpl.c | 18 ++++++++------
tools/xenstore/xenstored_core.c | 4 +++-
tools/xenstore/xenstored_domain.c | 2 +-
tools/xenstore/xs.c | 4 ++++
xen/common/grant_table.c | 1 -
23 files changed, 123 insertions(+), 69 deletions(-)
--
1.7.10.4
Matthew Daley
2013-Sep-18 03:37 UTC
[PATCH 01/28] gnttab: remove unused shared header lookup
Coverity-ID: 1056171
Signed-off-by: Matthew Daley <mattjd@gmail.com>
---
xen/common/grant_table.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
index eb50288..f42bc7a 100644
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -783,7 +783,6 @@ __gnttab_map_grant_ref(
spin_lock(&rgt->lock);
act = &active_entry(rgt, op->ref);
- shah = shared_entry_header(rgt, op->ref);
if ( op->flags & GNTMAP_device_map )
act->pin -= (op->flags & GNTMAP_readonly) ?
--
1.7.10.4
Matthew Daley
2013-Sep-18 03:37 UTC
[PATCH 02/28] libxc: fix memory leak in load_p2m_frame_list error handling
Coverity-ID: 1055885
Signed-off-by: Matthew Daley <mattjd@gmail.com>
---
tools/libxc/xc_domain_restore.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/libxc/xc_domain_restore.c b/tools/libxc/xc_domain_restore.c
index b418963..939a76b 100644
--- a/tools/libxc/xc_domain_restore.c
+++ b/tools/libxc/xc_domain_restore.c
@@ -368,6 +368,7 @@ static xen_pfn_t *load_p2m_frame_list(
(P2M_FL_ENTRIES - 1) * sizeof(xen_pfn_t)) )
{
PERROR("read p2m_frame_list failed");
+ free(p2m_frame_list);
return NULL;
}
--
1.7.10.4
Matthew Daley
2013-Sep-18 03:37 UTC
[PATCH 03/28] libxc: move munmap into the loop it''s needed in in change_pte
Coverity-ID: 1055269
Signed-off-by: Matthew Daley <mattjd@gmail.com>
---
tools/libxc/xc_offline_page.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/libxc/xc_offline_page.c b/tools/libxc/xc_offline_page.c
index fbb53f5..8195efb 100644
--- a/tools/libxc/xc_offline_page.c
+++ b/tools/libxc/xc_offline_page.c
@@ -317,10 +317,10 @@ static int change_pte(xc_interface *xch, int domid,
goto failed;
}
}
- }
- munmap(content, PAGE_SIZE);
- content = NULL;
+ munmap(content, PAGE_SIZE);
+ content = NULL;
+ }
}
if ( xc_flush_mmu_updates(xch, mmu) )
--
1.7.10.4
Matthew Daley
2013-Sep-18 03:37 UTC
[PATCH 04/28] libxl: fix libxl_string_list_length and its only caller
The wrong amount of indirections were being taken in
libxl_string_list_length, and its only caller was miscounting the amount
of initial non-list arguments, seemingly since the initial commit
(599c784).
This has been seen and reported in the wild (##xen):
< Trixboxer> Hi, any idea why would I get
< Trixboxer> xl: libxl_bootloader.c:42: bootloader_arg: Assertion
`bl->nargs < bl->argsspace'' failed.
< Trixboxer> 4.2.2-23.el6
Coverity-ID: 1054954
Signed-off-by: Matthew Daley <mattjd@gmail.com>
---
tools/libxl/libxl.c | 2 +-
tools/libxl/libxl_bootloader.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 0879f23..ca24ca3 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -202,7 +202,7 @@ int libxl_string_list_length(const libxl_string_list *psl)
{
if (!psl) return 0;
int i = 0;
- while (*psl++) i++;
+ while ((*psl)[i]) i++;
return i;
}
diff --git a/tools/libxl/libxl_bootloader.c b/tools/libxl/libxl_bootloader.c
index ed12b2c..3287bf7 100644
--- a/tools/libxl/libxl_bootloader.c
+++ b/tools/libxl/libxl_bootloader.c
@@ -48,7 +48,7 @@ static void make_bootloader_args(libxl__gc *gc,
libxl__bootloader_state *bl,
{
const libxl_domain_build_info *info = bl->info;
- bl->argsspace = 7 +
libxl_string_list_length(&info->u.pv.bootloader_args);
+ bl->argsspace = 9 +
libxl_string_list_length(&info->u.pv.bootloader_args);
GCNEW_ARRAY(bl->args, bl->argsspace);
--
1.7.10.4
Matthew Daley
2013-Sep-18 03:37 UTC
[PATCH 05/28] libxl: fix dispose without init of disk in cd_insert
Coverity-ID: 1056078
Signed-off-by: Matthew Daley <mattjd@gmail.com>
---
tools/libxl/xl_cmdimpl.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 3d7eaad..a609b98 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -2529,18 +2529,16 @@ int main_memset(int argc, char **argv)
static int cd_insert(uint32_t domid, const char *virtdev, char *phys)
{
- libxl_device_disk disk; /* we don''t free disk''s contents
*/
+ libxl_device_disk disk;
char *buf = NULL;
XLU_Config *config = 0;
struct stat b;
int rc = 0;
-
if (asprintf(&buf,
"vdev=%s,access=r,devtype=cdrom,target=%s",
virtdev, phys ? phys : "") < 0) {
fprintf(stderr, "out of memory\n");
- rc = 1;
- goto out;
+ return 1;
}
parse_disk_config(&config, buf, &disk);
--
1.7.10.4
Matthew Daley
2013-Sep-18 03:37 UTC
[PATCH 06/28] libxl: fix leak of corename in handle_domain_death
Coverity-ID: 1087192
Signed-off-by: Matthew Daley <mattjd@gmail.com>
---
tools/libxl/xl_cmdimpl.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index a609b98..211bdea 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -1621,6 +1621,7 @@ static int handle_domain_death(uint32_t *r_domid,
LOG("dumping core to %s", corefile);
rc=libxl_domain_core_dump(ctx, *r_domid, corefile, NULL);
if (rc) LOG("core dump failed (rc=%d).", rc);
+ free(corefile);
}
/* No point crying over spilled milk, continue on failure. */
--
1.7.10.4
Matthew Daley
2013-Sep-18 03:37 UTC
[PATCH 07/28] libxl: fix leak of config_data in main_cpupoolcreate
Coverity-ID: 1087193
Signed-off-by: Matthew Daley <mattjd@gmail.com>
---
tools/libxl/xl_cmdimpl.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 211bdea..b05763f 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -6554,6 +6554,7 @@ int main_cpupoolcreate(int argc, char **argv)
out_cfg:
xlu_cfg_destroy(config);
out:
+ free(config_data);
return rc;
}
--
1.7.10.4
Coverity-ID: 1087194
Signed-off-by: Matthew Daley <mattjd@gmail.com>
---
tools/libxl/xl_cmdimpl.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index b05763f..81ec66c 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -7125,6 +7125,9 @@ int main_remus(int argc, char **argv)
migrate_do_preamble(send_fd, recv_fd, child, config_data, config_len,
rune);
+
+ if (ssh_command[0])
+ free(rune);
}
/* Point of no return */
--
1.7.10.4
Matthew Daley
2013-Sep-18 03:37 UTC
[PATCH 09/28] libxl: fix out-of-memory check in parse_global_config
Coverity-ID: 1055174
Signed-off-by: Matthew Daley <mattjd@gmail.com>
---
tools/libxl/xl.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/libxl/xl.c b/tools/libxl/xl.c
index b965cab..657610b 100644
--- a/tools/libxl/xl.c
+++ b/tools/libxl/xl.c
@@ -117,8 +117,8 @@ static void parse_global_config(const char *configfile,
lockfile = strdup(XL_LOCK_FILE);
}
- if (!lockfile < 0) {
- fprintf(stderr, "failed to allocate lockdir \n");
+ if (!lockfile) {
+ fprintf(stderr, "failed to allocate lockdir\n");
exit(1);
}
--
1.7.10.4
Matthew Daley
2013-Sep-18 03:37 UTC
[PATCH 10/28] libxl: only free cpupoolinfo if necessary in libxl_list_cpupool
Coverity-ID: 1055291
Signed-off-by: Matthew Daley <mattjd@gmail.com>
---
tools/libxl/libxl.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index ca24ca3..eeaaee8 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -650,7 +650,8 @@ libxl_cpupoolinfo * libxl_list_cpupool(libxl_ctx *ctx, int
*nb_pool_out)
tmp = realloc(ptr, (i + 1) * sizeof(libxl_cpupoolinfo));
if (!tmp) {
LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "allocating cpupool
info");
- libxl_cpupoolinfo_list_free(ptr, i);
+ if (ptr)
+ libxl_cpupoolinfo_list_free(ptr, i);
goto out;
}
ptr = tmp;
--
1.7.10.4
Matthew Daley
2013-Sep-18 03:37 UTC
[PATCH 11/28] libxl: only put poller if already gotten in libxl_event_wait
Coverity-ID: 1055292
Signed-off-by: Matthew Daley <mattjd@gmail.com>
---
tools/libxl/libxl_event.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/libxl/libxl_event.c b/tools/libxl/libxl_event.c
index e42b371..fd2f280 100644
--- a/tools/libxl/libxl_event.c
+++ b/tools/libxl/libxl_event.c
@@ -1476,7 +1476,7 @@ int libxl_event_wait(libxl_ctx *ctx, libxl_event
**event_r,
}
out:
- libxl__poller_put(ctx, poller);
+ if (poller) libxl__poller_put(ctx, poller);
CTX_UNLOCK;
EGC_FREE;
--
1.7.10.4
Matthew Daley
2013-Sep-18 03:37 UTC
[PATCH 12/28] libxl: only free cputopology if it was allocated in libxl__get_numa_candidate
Coverity-ID: 1055293
Signed-off-by: Matthew Daley <mattjd@gmail.com>
---
tools/libxl/libxl_numa.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tools/libxl/libxl_numa.c b/tools/libxl/libxl_numa.c
index 20c99ac..f9944e0 100644
--- a/tools/libxl/libxl_numa.c
+++ b/tools/libxl/libxl_numa.c
@@ -495,7 +495,8 @@ int libxl__get_numa_candidate(libxl__gc *gc,
libxl_bitmap_dispose(&suitable_nodemap);
libxl__numa_candidate_dispose(&new_cndt);
libxl_numainfo_list_free(ninfo, nr_nodes);
- libxl_cputopology_list_free(tinfo, nr_cpus);
+ if (tinfo != NULL)
+ libxl_cputopology_list_free(tinfo, nr_cpus);
return rc;
}
--
1.7.10.4
Matthew Daley
2013-Sep-18 03:37 UTC
[PATCH 13/28] libxl: only free cputopology if it was allocated in libxl_{cpu, node}map_to_{node, cpu}map
Coverity-ID: 1055294
Coverity-ID: 1055295
Signed-off-by: Matthew Daley <mattjd@gmail.com>
---
tools/libxl/libxl_utils.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
index 4309e5e..8e567a8 100644
--- a/tools/libxl/libxl_utils.c
+++ b/tools/libxl/libxl_utils.c
@@ -664,7 +664,8 @@ int libxl_nodemap_to_cpumap(libxl_ctx *ctx,
libxl_bitmap_set(cpumap, i);
}
out:
- libxl_cputopology_list_free(tinfo, nr_cpus);
+ if (tinfo != NULL)
+ libxl_cputopology_list_free(tinfo, nr_cpus);
return rc;
}
@@ -685,7 +686,8 @@ int libxl_cpumap_to_nodemap(libxl_ctx *ctx,
libxl_for_each_set_bit(i, *cpumap)
libxl_bitmap_set(nodemap, tinfo[i].node);
out:
- libxl_cputopology_list_free(tinfo, nr_cpus);
+ if (tinfo != NULL)
+ libxl_cputopology_list_free(tinfo, nr_cpus);
return rc;
}
--
1.7.10.4
Matthew Daley
2013-Sep-18 03:37 UTC
[PATCH 14/28] libxl: only free console reader if it was allocated in main_dmesg
Coverity-ID: 1055304
Signed-off-by: Matthew Daley <mattjd@gmail.com>
---
tools/libxl/xl_cmdimpl.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 81ec66c..642b130 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -5537,7 +5537,8 @@ int main_dmesg(int argc, char **argv)
printf("%s", line);
finish:
- libxl_xen_console_read_finish(ctx, cr);
+ if (cr)
+ libxl_xen_console_read_finish(ctx, cr);
return ret;
}
--
1.7.10.4
Matthew Daley
2013-Sep-18 03:37 UTC
[PATCH 15/28] libxl: fix typo in libxl__hotplug_nic error checking
Coverity-ID: 1055945
Signed-off-by: Matthew Daley <mattjd@gmail.com>
---
tools/libxl/libxl_linux.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/libxl/libxl_linux.c b/tools/libxl/libxl_linux.c
index 37815eb..ea5d8c1 100644
--- a/tools/libxl/libxl_linux.c
+++ b/tools/libxl/libxl_linux.c
@@ -170,7 +170,7 @@ static int libxl__hotplug_nic(libxl__gc *gc, libxl__device
*dev,
}
*env = get_hotplug_env(gc, script, dev);
- if (!env) {
+ if (!*env) {
rc = ERROR_FAIL;
goto out;
}
--
1.7.10.4
Matthew Daley
2013-Sep-18 03:37 UTC
[PATCH 16/28] libxl: fix file open failure check in libxl__file_reference_map
Coverity-ID: 1055567
Signed-off-by: Matthew Daley <mattjd@gmail.com>
---
tools/libxl/libxl_internal.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/libxl/libxl_internal.c b/tools/libxl/libxl_internal.c
index 5a8cd38..cf17658 100644
--- a/tools/libxl/libxl_internal.c
+++ b/tools/libxl/libxl_internal.c
@@ -228,7 +228,7 @@ int libxl__file_reference_map(libxl__file_reference *f)
return 0;
fd = open(f->path, O_RDONLY);
- if (f < 0)
+ if (fd < 0)
return ERROR_FAIL;
ret = fstat(fd, &st_buf);
--
1.7.10.4
Matthew Daley
2013-Sep-18 03:37 UTC
[PATCH 17/28] libxl: gettimeofday doesn''t return an errno on failure
Coverity-ID: 1055570
Signed-off-by: Matthew Daley <mattjd@gmail.com>
---
tools/libxl/xl_cmdimpl.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 642b130..7ec5d6a 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -3392,7 +3392,7 @@ static void migration_child_report(int recv_fd) {
if (!xl_child_pid(child_migration)) return;
- CHK_ERRNO( gettimeofday(&waituntil, 0) );
+ MUST( gettimeofday(&waituntil, 0) );
waituntil.tv_sec += 2;
for (;;) {
@@ -3413,7 +3413,7 @@ static void migration_child_report(int recv_fd) {
}
assert(child == 0);
- CHK_ERRNO( gettimeofday(&now, 0) );
+ MUST( gettimeofday(&now, 0) );
if (timercmp(&now, &waituntil, >)) {
fprintf(stderr, "migration child [%ld] not exiting, no
longer"
" waiting (exit status will be unreported)\n",
--
1.7.10.4
Matthew Daley
2013-Sep-18 03:37 UTC
[PATCH 18/28] xenstored: handle unlikely failure better in ask_parents
Coverity-ID: 1055277
Signed-off-by: Matthew Daley <mattjd@gmail.com>
---
tools/xenstore/xenstored_core.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index a2cf2a6..0f8ba64 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -523,8 +523,10 @@ static enum xs_perm_type ask_parents(struct connection
*conn, const char *name)
} while (!streq(name, "/"));
/* No permission at root? We''re in trouble. */
- if (!node)
+ if (!node) {
corrupt(conn, "No permissions file at root");
+ return XS_PERM_NONE;
+ }
return perm_for_conn(conn, node->perms, node->num_perms);
}
--
1.7.10.4
Matthew Daley
2013-Sep-18 03:37 UTC
[PATCH 19/28] xenstored: fix faulty check for bad handle in domain_init
Coverity-ID: 1054975
Coverity-ID: 1055196
Signed-off-by: Matthew Daley <mattjd@gmail.com>
---
tools/xenstore/xenstored_domain.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/xenstore/xenstored_domain.c
b/tools/xenstore/xenstored_domain.c
index bf83d58..f24bd6b 100644
--- a/tools/xenstore/xenstored_domain.c
+++ b/tools/xenstore/xenstored_domain.c
@@ -640,7 +640,7 @@ void domain_init(void)
barf_perror("Failed to allocate domain gnttab handle");
*xcg_handle = xc_gnttab_open(NULL, 0);
- if (*xcg_handle < 0)
+ if (*xcg_handle == NULL)
xprintf("WARNING: Failed to open connection to gnttab\n");
else
talloc_set_destructor(xcg_handle, close_xcg_handle);
--
1.7.10.4
Matthew Daley
2013-Sep-18 03:37 UTC
[PATCH 20/28] xenstore: check socket path length before copying it
Coverity-ID: 1055997
Signed-off-by: Matthew Daley <mattjd@gmail.com>
---
tools/xenstore/xs.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/tools/xenstore/xs.c b/tools/xenstore/xs.c
index 86ef6c7..b1e6820 100644
--- a/tools/xenstore/xs.c
+++ b/tools/xenstore/xs.c
@@ -196,6 +196,10 @@ static int get_socket(const char *connect_to)
goto error;
addr.sun_family = AF_UNIX;
+ if(strlen(connect_to) >= sizeof(addr.sun_path)) {
+ errno = EINVAL;
+ goto error;
+ }
strcpy(addr.sun_path, connect_to);
if (connect(sock, (struct sockaddr *)&addr, sizeof(addr)) != 0)
--
1.7.10.4
Matthew Daley
2013-Sep-18 03:37 UTC
[PATCH 21/28] mini-os: fix nodename generation in init_netfront
Using strlen here makes no sense.
Coverity-ID: 1056053
Signed-off-by: Matthew Daley <mattjd@gmail.com>
---
extras/mini-os/netfront.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/extras/mini-os/netfront.c b/extras/mini-os/netfront.c
index 4e087a5..a999985 100644
--- a/extras/mini-os/netfront.c
+++ b/extras/mini-os/netfront.c
@@ -309,8 +309,10 @@ struct netfront_dev *init_netfront(char *_nodename, void
(*thenetif_rx)(unsigned
if (!_nodename)
snprintf(nodename, sizeof(nodename), "device/vif/%d",
netfrontends);
- else
- strncpy(nodename, _nodename, strlen(nodename));
+ else {
+ strncpy(nodename, _nodename, sizeof(nodename) - 1);
+ nodename[sizeof(nodename) - 1] = 0;
+ }
netfrontends++;
if (!thenetif_rx)
--
1.7.10.4
Matthew Daley
2013-Sep-18 03:37 UTC
[PATCH 22/28] mini-os: fix various memory leaks in blkfront
Coverity-ID: 1055814
Coverity-ID: 1055815
Signed-off-by: Matthew Daley <mattjd@gmail.com>
---
extras/mini-os/blkfront.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/extras/mini-os/blkfront.c b/extras/mini-os/blkfront.c
index ddcf665..aead6bd 100644
--- a/extras/mini-os/blkfront.c
+++ b/extras/mini-os/blkfront.c
@@ -250,7 +250,7 @@ error:
void shutdown_blkfront(struct blkfront_dev *dev)
{
- char* err = NULL;
+ char* err = NULL, *err2;
XenbusState state;
char path[strlen(dev->backend) + strlen("/state") + 1];
@@ -295,12 +295,15 @@ void shutdown_blkfront(struct blkfront_dev *dev)
close:
if (err) free(err);
- xenbus_unwatch_path_token(XBT_NIL, path, path);
+ err2 = xenbus_unwatch_path_token(XBT_NIL, path, path);
+ if (err2) free(err2);
snprintf(nodename, sizeof(nodename), "%s/ring-ref",
dev->nodename);
- xenbus_rm(XBT_NIL, nodename);
+ err2 = xenbus_rm(XBT_NIL, nodename);
+ if (err2) free(err2);
snprintf(nodename, sizeof(nodename), "%s/event-channel",
dev->nodename);
- xenbus_rm(XBT_NIL, nodename);
+ err2 = xenbus_rm(XBT_NIL, nodename);
+ if (err2) free(err2);
if (!err)
free_blkfront(dev);
--
1.7.10.4
Matthew Daley
2013-Sep-18 03:37 UTC
[PATCH 23/28] mini-os: fix various memory leaks in {fb, kbd}front
Coverity-ID: 1055819-1055826
Signed-off-by: Matthew Daley <mattjd@gmail.com>
---
extras/mini-os/fbfront.c | 48 ++++++++++++++++++++++++++++++----------------
1 file changed, 31 insertions(+), 17 deletions(-)
diff --git a/extras/mini-os/fbfront.c b/extras/mini-os/fbfront.c
index 3d0b8f5..a276193 100644
--- a/extras/mini-os/fbfront.c
+++ b/extras/mini-os/fbfront.c
@@ -171,7 +171,8 @@ done:
err = xenbus_wait_for_state_change(path, &state,
&dev->events);
if (state != XenbusStateConnected) {
printk("backend not available, state=%d\n", state);
- xenbus_unwatch_path_token(XBT_NIL, path, path);
+ if (err) free(err);
+ err = xenbus_unwatch_path_token(XBT_NIL, path, path);
goto error;
}
@@ -181,7 +182,8 @@ done:
if((err = xenbus_switch_state(XBT_NIL, frontpath,
XenbusStateConnected))
!= NULL) {
printk("error switching state: %s\n", err);
- xenbus_unwatch_path_token(XBT_NIL, path, path);
+ free(err);
+ err = xenbus_unwatch_path_token(XBT_NIL, path, path);
goto error;
}
}
@@ -236,7 +238,7 @@ int kbdfront_receive(struct kbdfront_dev *dev, union
xenkbd_in_event *buf, int n
void shutdown_kbdfront(struct kbdfront_dev *dev)
{
- char* err = NULL;
+ char* err = NULL, *err2;
XenbusState state;
char path[strlen(dev->backend) + strlen("/state") + 1];
@@ -278,14 +280,18 @@ void shutdown_kbdfront(struct kbdfront_dev *dev)
close_kbdfront:
if (err) free(err);
- xenbus_unwatch_path_token(XBT_NIL, path, path);
+ err2 = xenbus_unwatch_path_token(XBT_NIL, path, path);
+ if (err2) free(err2);
snprintf(nodename, sizeof(nodename), "%s/page-ref",
dev->nodename);
- xenbus_rm(XBT_NIL, nodename);
+ err2 = xenbus_rm(XBT_NIL, nodename);
+ if (err2) free(err2);
snprintf(nodename, sizeof(nodename), "%s/event-channel",
dev->nodename);
- xenbus_rm(XBT_NIL, nodename);
+ err2 = xenbus_rm(XBT_NIL, nodename);
+ if (err2) free(err2);
snprintf(nodename, sizeof(nodename), "%s/request-abs-pointer",
dev->nodename);
- xenbus_rm(XBT_NIL, nodename);
+ err2 = xenbus_rm(XBT_NIL, nodename);
+ if (err2) free(err2);
if (!err)
free_kbdfront(dev);
@@ -405,7 +411,7 @@ struct fbfront_dev *init_fbfront(char *_nodename, unsigned
long *mfns, int width
char* message=NULL;
struct xenfb_page *s;
int retry=0;
- char* msg;
+ char* msg=NULL;
int i, j;
struct fbfront_dev *dev;
int max_pd;
@@ -532,7 +538,8 @@ done:
err = xenbus_wait_for_state_change(path, &state,
&dev->events);
if (state != XenbusStateConnected) {
printk("backend not available, state=%d\n", state);
- xenbus_unwatch_path_token(XBT_NIL, path, path);
+ if (err) free(err);
+ err = xenbus_unwatch_path_token(XBT_NIL, path, path);
goto error;
}
@@ -545,7 +552,8 @@ done:
if ((err = xenbus_switch_state(XBT_NIL, frontpath,
XenbusStateConnected))
!= NULL) {
printk("error switching state: %s\n", err);
- xenbus_unwatch_path_token(XBT_NIL, path, path);
+ free(err);
+ err = xenbus_unwatch_path_token(XBT_NIL, path, path);
goto error;
}
}
@@ -556,6 +564,7 @@ done:
return dev;
error:
+ free(msg);
free(err);
free_fbfront(dev);
return NULL;
@@ -627,7 +636,7 @@ void fbfront_resize(struct fbfront_dev *dev, int width, int
height, int stride,
void shutdown_fbfront(struct fbfront_dev *dev)
{
- char* err = NULL;
+ char* err = NULL, *err2;
XenbusState state;
char path[strlen(dev->backend) + strlen("/state") + 1];
@@ -654,7 +663,7 @@ void shutdown_fbfront(struct fbfront_dev *dev)
}
state = xenbus_read_integer(path);
if (state < XenbusStateClosed) {
- xenbus_wait_for_state_change(path, &state, &dev->events);
+ err = xenbus_wait_for_state_change(path, &state,
&dev->events);
if (err) free(err);
}
@@ -669,16 +678,21 @@ void shutdown_fbfront(struct fbfront_dev *dev)
close_fbfront:
if (err) free(err);
- xenbus_unwatch_path_token(XBT_NIL, path, path);
+ err2 = xenbus_unwatch_path_token(XBT_NIL, path, path);
+ if (err2) free(err2);
snprintf(nodename, sizeof(nodename), "%s/page-ref",
dev->nodename);
- xenbus_rm(XBT_NIL, nodename);
+ err2 = xenbus_rm(XBT_NIL, nodename);
+ if (err2) free(err2);
snprintf(nodename, sizeof(nodename), "%s/event-channel",
dev->nodename);
- xenbus_rm(XBT_NIL, nodename);
+ err2 = xenbus_rm(XBT_NIL, nodename);
+ if (err2) free(err2);
snprintf(nodename, sizeof(nodename), "%s/protocol",
dev->nodename);
- xenbus_rm(XBT_NIL, nodename);
+ err2 = xenbus_rm(XBT_NIL, nodename);
+ if (err2) free(err2);
snprintf(nodename, sizeof(nodename), "%s/feature-update",
dev->nodename);
- xenbus_rm(XBT_NIL, nodename);
+ err2 = xenbus_rm(XBT_NIL, nodename);
+ if (err2) free(err2);
if (!err)
free_fbfront(dev);
--
1.7.10.4
Matthew Daley
2013-Sep-18 03:38 UTC
[PATCH 24/28] mini-os: fix various memory leaks in netfront
Coverity-ID: 1055832
Coverity-ID: 1055833
Signed-off-by: Matthew Daley <mattjd@gmail.com>
---
extras/mini-os/netfront.c | 17 +++++++++++------
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/extras/mini-os/netfront.c b/extras/mini-os/netfront.c
index a999985..3a5be64 100644
--- a/extras/mini-os/netfront.c
+++ b/extras/mini-os/netfront.c
@@ -506,7 +506,7 @@ int netfront_tap_open(char *nodename) {
void shutdown_netfront(struct netfront_dev *dev)
{
- char* err = NULL;
+ char* err = NULL, *err2;
XenbusState state;
char path[strlen(dev->backend) + strlen("/state") + 1];
@@ -549,16 +549,21 @@ void shutdown_netfront(struct netfront_dev *dev)
close:
if (err) free(err);
- xenbus_unwatch_path_token(XBT_NIL, path, path);
+ err2 = xenbus_unwatch_path_token(XBT_NIL, path, path);
+ if (err2) free(err2);
snprintf(nodename, sizeof(nodename), "%s/tx-ring-ref",
dev->nodename);
- xenbus_rm(XBT_NIL, nodename);
+ err2 = xenbus_rm(XBT_NIL, nodename);
+ if (err2) free(err2);
snprintf(nodename, sizeof(nodename), "%s/rx-ring-ref",
dev->nodename);
- xenbus_rm(XBT_NIL, nodename);
+ err2 = xenbus_rm(XBT_NIL, nodename);
+ if (err2) free(err2);
snprintf(nodename, sizeof(nodename), "%s/event-channel",
dev->nodename);
- xenbus_rm(XBT_NIL, nodename);
+ err2 = xenbus_rm(XBT_NIL, nodename);
+ if (err2) free(err2);
snprintf(nodename, sizeof(nodename), "%s/request-rx-copy",
dev->nodename);
- xenbus_rm(XBT_NIL, nodename);
+ err2 = xenbus_rm(XBT_NIL, nodename);
+ if (err2) free(err2);
if (!err)
free_netfront(dev);
--
1.7.10.4
Matthew Daley
2013-Sep-18 03:38 UTC
[PATCH 25/28] mini-os: fix various memory leaks in pcifront
Coverity-ID: 1055834-1055840
Signed-off-by: Matthew Daley <mattjd@gmail.com>
---
extras/mini-os/pcifront.c | 29 +++++++++++++++++++----------
1 file changed, 19 insertions(+), 10 deletions(-)
diff --git a/extras/mini-os/pcifront.c b/extras/mini-os/pcifront.c
index cdf9c9b..7838021 100644
--- a/extras/mini-os/pcifront.c
+++ b/extras/mini-os/pcifront.c
@@ -122,11 +122,12 @@ void pcifront_watches(void *opaque)
} else if (state == XenbusStateClosing)
break;
}
- if (err)
+ if (err) {
printk("pcifront_watches: done waiting err=%s\n", err);
- else
+ free(err);
+ } else
printk("pcifront_watches: done waiting\n");
- xenbus_unwatch_path_token(XBT_NIL, be_state, be_state);
+ err = xenbus_unwatch_path_token(XBT_NIL, be_state, be_state);
shutdown_pcifront(pcidev);
free(be_state);
free(be_path);
@@ -143,7 +144,7 @@ struct pcifront_dev *init_pcifront(char *_nodename)
char* err;
char* message=NULL;
int retry=0;
- char* msg;
+ char* msg = NULL;
char* nodename = _nodename ? _nodename : "device/pci/0";
int dom;
@@ -250,7 +251,8 @@ done:
err = xenbus_wait_for_state_change(path, &state,
&dev->events);
if (state != XenbusStateConnected) {
printk("backend not avalable, state=%d\n", state);
- xenbus_unwatch_path_token(XBT_NIL, path, path);
+ if (err) free(err);
+ err = xenbus_unwatch_path_token(XBT_NIL, path, path);
goto error;
}
@@ -258,7 +260,8 @@ done:
if ((err = xenbus_switch_state(XBT_NIL, frontpath,
XenbusStateConnected))
!= NULL) {
printk("error switching state %s\n", err);
- xenbus_unwatch_path_token(XBT_NIL, path, path);
+ free(err);
+ err = xenbus_unwatch_path_token(XBT_NIL, path, path);
goto error;
}
}
@@ -272,6 +275,7 @@ done:
return dev;
error:
+ free(msg);
free(err);
free_pcifront(dev);
return NULL;
@@ -301,6 +305,7 @@ void pcifront_scan(struct pcifront_dev *dev, void
(*func)(unsigned int domain, u
msg = xenbus_read(XBT_NIL, path, &s);
if (msg) {
printk("Error %s when reading the PCI root name at %s\n",
msg, path);
+ free(msg);
continue;
}
@@ -319,7 +324,7 @@ void pcifront_scan(struct pcifront_dev *dev, void
(*func)(unsigned int domain, u
void shutdown_pcifront(struct pcifront_dev *dev)
{
- char* err = NULL;
+ char* err = NULL, *err2;
XenbusState state;
char path[strlen(dev->backend) + strlen("/state") + 1];
@@ -361,12 +366,15 @@ void shutdown_pcifront(struct pcifront_dev *dev)
close_pcifront:
if (err) free(err);
- xenbus_unwatch_path_token(XBT_NIL, path, path);
+ err2 = xenbus_unwatch_path_token(XBT_NIL, path, path);
+ if (err2) free(err2);
snprintf(nodename, sizeof(nodename), "%s/info-ref",
dev->nodename);
- xenbus_rm(XBT_NIL, nodename);
+ err2 = xenbus_rm(XBT_NIL, nodename);
+ if (err2) free(err2);
snprintf(nodename, sizeof(nodename), "%s/event-channel",
dev->nodename);
- xenbus_rm(XBT_NIL, nodename);
+ err2 = xenbus_rm(XBT_NIL, nodename);
+ if (err2) free(err2);
if (!err)
free_pcifront(dev);
@@ -397,6 +405,7 @@ int pcifront_physical_to_virtual (struct pcifront_dev *dev,
msg = xenbus_read(XBT_NIL, path, &s);
if (msg) {
printk("Error %s when reading the PCI root name at %s\n",
msg, path);
+ free(msg);
continue;
}
--
1.7.10.4
Matthew Daley
2013-Sep-18 03:38 UTC
[PATCH 26/28] mini-os: fix various memory leaks in consfront
Coverity-ID: 1055816
Coverity-ID: 1055817
Coverity-ID: 1055818
Signed-off-by: Matthew Daley <mattjd@gmail.com>
---
extras/mini-os/console/xenbus.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/extras/mini-os/console/xenbus.c b/extras/mini-os/console/xenbus.c
index 4194982..b317114 100644
--- a/extras/mini-os/console/xenbus.c
+++ b/extras/mini-os/console/xenbus.c
@@ -42,7 +42,8 @@ void free_consfront(struct consfront_dev *dev)
close:
if (err) free(err);
- xenbus_unwatch_path_token(XBT_NIL, path, path);
+ err = xenbus_unwatch_path_token(XBT_NIL, path, path);
+ if (err) free(err);
mask_evtchn(dev->evtchn);
unbind_evtchn(dev->evtchn);
@@ -58,7 +59,7 @@ close:
struct consfront_dev *init_consfront(char *_nodename)
{
xenbus_transaction_t xbt;
- char* err;
+ char* err = NULL;
char* message=NULL;
int retry=0;
char* msg = NULL;
@@ -87,7 +88,7 @@ struct consfront_dev *init_consfront(char *_nodename)
snprintf(path, sizeof(path), "%s/backend-id", nodename);
if ((res = xenbus_read_integer(path)) < 0)
- return NULL;
+ goto error;
else
dev->dom = res;
evtchn_alloc_unbound(dev->dom, console_handle_input, dev,
&dev->evtchn);
@@ -170,7 +171,7 @@ done:
msg = xenbus_wait_for_state_change(path, &state,
&dev->events);
if (msg != NULL || state != XenbusStateConnected) {
printk("backend not available, state=%d\n", state);
- xenbus_unwatch_path_token(XBT_NIL, path, path);
+ err = xenbus_unwatch_path_token(XBT_NIL, path, path);
goto error;
}
}
--
1.7.10.4
Matthew Daley
2013-Sep-18 03:38 UTC
[PATCH 27/28] mini-os: fix various memory leaks in various locations
Coverity-ID: 1055827
Coverity-ID: 1055828
Coverity-ID: 1055829
Coverity-ID: 1055830
Coverity-ID: 1055831
Signed-off-by: Matthew Daley <mattjd@gmail.com>
---
extras/mini-os/kernel.c | 7 +++++--
extras/mini-os/lib/xs.c | 2 ++
2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/extras/mini-os/kernel.c b/extras/mini-os/kernel.c
index e9402b9..24fa25c 100644
--- a/extras/mini-os/kernel.c
+++ b/extras/mini-os/kernel.c
@@ -84,8 +84,10 @@ static void shutdown_thread(void *p)
free(err);
xenbus_wait_for_watch(&events);
}
- xenbus_unwatch_path_token(XBT_NIL, path, token);
- xenbus_write(XBT_NIL, path, "");
+ err = xenbus_unwatch_path_token(XBT_NIL, path, token);
+ if (err) free(err);
+ err = xenbus_write(XBT_NIL, path, "");
+ if (err) free(err);
printk("Shutting down (%s)\n", shutdown);
if (!strcmp(shutdown, "poweroff"))
@@ -96,6 +98,7 @@ static void shutdown_thread(void *p)
/* Unknown */
shutdown_reason = SHUTDOWN_crash;
app_shutdown(shutdown_reason);
+ free(shutdown);
}
#endif
diff --git a/extras/mini-os/lib/xs.c b/extras/mini-os/lib/xs.c
index c603d17..324bd05 100644
--- a/extras/mini-os/lib/xs.c
+++ b/extras/mini-os/lib/xs.c
@@ -144,6 +144,7 @@ char **xs_directory(struct xs_handle *h, xs_transaction_t t,
msg = xenbus_ls(t, path, &res);
if (msg) {
printk("xs_directory(%s): %s\n", path, msg);
+ free(msg);
return NULL;
}
@@ -163,6 +164,7 @@ char **xs_directory(struct xs_handle *h, xs_transaction_t t,
}
*num = n;
+ free(res);
return entries;
}
--
1.7.10.4
...so that sin_zero is actually zero.
Coverity-ID: 1056070
Signed-off-by: Matthew Daley <mattjd@gmail.com>
---
tools/debugger/gdbsx/gx/gx_comm.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/debugger/gdbsx/gx/gx_comm.c
b/tools/debugger/gdbsx/gx/gx_comm.c
index ed4a7be..7680dbd 100644
--- a/tools/debugger/gdbsx/gx/gx_comm.c
+++ b/tools/debugger/gdbsx/gx/gx_comm.c
@@ -84,6 +84,7 @@ do_tcp(char *port_str)
tmp = 1;
setsockopt(sock_fd, SOL_SOCKET, SO_REUSEADDR, (char
*)&tmp,sizeof(tmp));
+ memset(&sockaddr, 0, sizeof(sockaddr));
sockaddr.sin_family = PF_INET;
sockaddr.sin_port = htons (port);
sockaddr.sin_addr.s_addr = INADDR_ANY;
--
1.7.10.4
Dario Faggioli
2013-Sep-18 07:57 UTC
Re: [PATCH 12/28] libxl: only free cputopology if it was allocated in libxl__get_numa_candidate
On mer, 2013-09-18 at 15:37 +1200, Matthew Daley wrote:> Coverity-ID: 1055293 > Signed-off-by: Matthew Daley <mattjd@gmail.com> > --- > > diff --git a/tools/libxl/libxl_numa.c b/tools/libxl/libxl_numa.c > index 20c99ac..f9944e0 100644 > --- a/tools/libxl/libxl_numa.c > +++ b/tools/libxl/libxl_numa.c > @@ -495,7 +495,8 @@ int libxl__get_numa_candidate(libxl__gc *gc, > libxl_bitmap_dispose(&suitable_nodemap); > libxl__numa_candidate_dispose(&new_cndt); > libxl_numainfo_list_free(ninfo, nr_nodes); > - libxl_cputopology_list_free(tinfo, nr_cpus); > + if (tinfo != NULL) > + libxl_cputopology_list_free(tinfo, nr_cpus); >I don''t think this is necessary. Actually, although not wrong, it deviates from the usual exiting pattern: init everything at the beginning, free everything on the way out. libxl_cputopology_list_free() is well capable of dealing with a non-allocated tinfo, provided nr_cpus is 0 too. If I''m not mistaken, that is exactly the case, all the times that tinfo is NULL, since: - it is initialized to 0 in libxl__get_numa_candidate(); - its value is only altered, within libxl_get_cpu_topology(), in case tinfo != NULL So, really, I don''t think we should apply this. Also, if something like that would be necessary, why doing it only for tinfo and not for ninfo as well? I''m not that into coverity, but I don''t think I see why it treats the two of them differently... :-O Regards, Dario -- <<This happens because I choose it to happen!>> (Raistlin Majere) ----------------------------------------------------------------- Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK) _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Dario Faggioli
2013-Sep-18 08:06 UTC
Re: [PATCH 10/28] libxl: only free cpupoolinfo if necessary in libxl_list_cpupool
[Adding Juergen] On mer, 2013-09-18 at 15:37 +1200, Matthew Daley wrote:> Coverity-ID: 1055291 > Signed-off-by: Matthew Daley <mattjd@gmail.com> > --- > tools/libxl/libxl.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c > index ca24ca3..eeaaee8 100644 > --- a/tools/libxl/libxl.c > +++ b/tools/libxl/libxl.c > @@ -650,7 +650,8 @@ libxl_cpupoolinfo * libxl_list_cpupool(libxl_ctx *ctx, int *nb_pool_out) > tmp = realloc(ptr, (i + 1) * sizeof(libxl_cpupoolinfo)); > if (!tmp) { > LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "allocating cpupool info"); > - libxl_cpupoolinfo_list_free(ptr, i); > + if (ptr) > + libxl_cpupoolinfo_list_free(ptr, i); >I''m less confident about libxl_cpupoolinfo_list_* than about topology and numa info handling via libxl, but I suspect this is something similar to what I just said in my reply to 12/28 ("libxl: only free cputopology if it was allocated in libxl__get_numa_candidate"). That is to say, this does not look necessary to me, as libxl_cpupoolinfo_list_free() seems to cope well with a non-allocated prt, and making it like this deviates from the usual libxl exit pattern/error handling style. Thoughts? Regards, Dario -- <<This happens because I choose it to happen!>> (Raistlin Majere) ----------------------------------------------------------------- Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK) _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Dario Faggioli
2013-Sep-18 08:18 UTC
Re: [PATCH 13/28] libxl: only free cputopology if it was allocated in libxl_{cpu, node}map_to_{node, cpu}map
On mer, 2013-09-18 at 15:37 +1200, Matthew Daley wrote:> Coverity-ID: 1055294 > Coverity-ID: 1055295 >Ok, forgive my coverity ignorance and sorry if I should know this already, but I do I use these IDs to get some useful information?> Signed-off-by: Matthew Daley <mattjd@gmail.com> > --- > tools/libxl/libxl_utils.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c > index 4309e5e..8e567a8 100644 > --- a/tools/libxl/libxl_utils.c > +++ b/tools/libxl/libxl_utils.c > @@ -664,7 +664,8 @@ int libxl_nodemap_to_cpumap(libxl_ctx *ctx, > libxl_bitmap_set(cpumap, i); > } > out: > - libxl_cputopology_list_free(tinfo, nr_cpus); > + if (tinfo != NULL) > + libxl_cputopology_list_free(tinfo, nr_cpus); > return rc; > }I was about to reply exactly as in the other e-mail (the reply to 12/28), but in this case I think there is some small rom for issues, since nr_cpus is not initialized. In fact, libxl_nodemap_to_cpumap() looks like this: 648 it libxl_nodemap_to_cpumap(libxl_ctx *ctx, 649 const libxl_bitmap *nodemap, 650 libxl_bitmap *cpumap) 651 { 652 libxl_cputopology *tinfo = NULL; 653 int nr_cpus, i, rc = 0; 654 655 tinfo = libxl_get_cpu_topology(ctx, &nr_cpus); 656 if (tinfo == NULL) { 657 rc = ERROR_FAIL; 658 goto out; 659 } 660 661 libxl_bitmap_set_none(cpumap); 662 for (i = 0; i < nr_cpus; i++) { 663 if (libxl_bitmap_test(nodemap, tinfo[i].node)) 664 libxl_bitmap_set(cpumap, i); 665 } 666 out: 667 libxl_cputopology_list_free(tinfo, nr_cpus); 668 return rc; 669 } Still, I think I''d like this fixed by doing something like this: 653 int nr_cpus = 0, i, rc = 0; rather than with what this patch does, for the same reasons already explained in the other e-mails. I''m up for sending such patch myself, but that will be in a while, as right now I''m away from my workstation. Is that fine with you?> > @@ -685,7 +686,8 @@ int libxl_cpumap_to_nodemap(libxl_ctx *ctx, > libxl_for_each_set_bit(i, *cpumap) > libxl_bitmap_set(nodemap, tinfo[i].node); > out: > - libxl_cputopology_list_free(tinfo, nr_cpus); > + if (tinfo != NULL) > + libxl_cputopology_list_free(tinfo, nr_cpus); > return rc; > } >And the same applies here, of course. Is that fine with you guys? Regards, Dario -- <<This happens because I choose it to happen!>> (Raistlin Majere) ----------------------------------------------------------------- Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK) _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Matthew Daley
2013-Sep-18 08:42 UTC
Re: [PATCH 12/28] libxl: only free cputopology if it was allocated in libxl__get_numa_candidate
On Wed, Sep 18, 2013 at 7:57 PM, Dario Faggioli <dario.faggioli@citrix.com> wrote:> On mer, 2013-09-18 at 15:37 +1200, Matthew Daley wrote: >> Coverity-ID: 1055293 >> Signed-off-by: Matthew Daley <mattjd@gmail.com> >> --- >> >> diff --git a/tools/libxl/libxl_numa.c b/tools/libxl/libxl_numa.c >> index 20c99ac..f9944e0 100644 >> --- a/tools/libxl/libxl_numa.c >> +++ b/tools/libxl/libxl_numa.c >> @@ -495,7 +495,8 @@ int libxl__get_numa_candidate(libxl__gc *gc, >> libxl_bitmap_dispose(&suitable_nodemap); >> libxl__numa_candidate_dispose(&new_cndt); >> libxl_numainfo_list_free(ninfo, nr_nodes); >> - libxl_cputopology_list_free(tinfo, nr_cpus); >> + if (tinfo != NULL) >> + libxl_cputopology_list_free(tinfo, nr_cpus); >> > I don''t think this is necessary. Actually, although not wrong, it > deviates from the usual exiting pattern: init everything at the > beginning, free everything on the way out. > > libxl_cputopology_list_free() is well capable of dealing with a > non-allocated tinfo, provided nr_cpus is 0 too. If I''m not mistaken, > that is exactly the case, all the times that tinfo is NULL, since: > - it is initialized to 0 in libxl__get_numa_candidate(); > - its value is only altered, within libxl_get_cpu_topology(), in case > tinfo != NULLArgh, you are absolutely correct.> > So, really, I don''t think we should apply this.Agreed. Please ignore this patch, and sorry for the noise.> > Also, if something like that would be necessary, why doing it only for > tinfo and not for ninfo as well? I''m not that into coverity, but I don''t > think I see why it treats the two of them differently... :-ONot sure. Probably because if ninfo == NULL, we return directly out of the function instead of goto-ing the cleanup. - Matthew
Matthew Daley
2013-Sep-18 08:50 UTC
Re: [PATCH 13/28] libxl: only free cputopology if it was allocated in libxl_{cpu, node}map_to_{node, cpu}map
On Wed, Sep 18, 2013 at 8:18 PM, Dario Faggioli <dario.faggioli@citrix.com> wrote:> On mer, 2013-09-18 at 15:37 +1200, Matthew Daley wrote: >> Coverity-ID: 1055294 >> Coverity-ID: 1055295 >> > Ok, forgive my coverity ignorance and sorry if I should know this > already, but I do I use these IDs to get some useful information?You can if you have access to the scan results. I''ve just been adding them to the commit messages as that''s what I''ve seen done in other projects (and other people''s Coverity-related Xen patches here). It also helps in situations like these.> >> Signed-off-by: Matthew Daley <mattjd@gmail.com> >> --- >> tools/libxl/libxl_utils.c | 6 ++++-- >> 1 file changed, 4 insertions(+), 2 deletions(-) >> >> diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c >> index 4309e5e..8e567a8 100644 >> --- a/tools/libxl/libxl_utils.c >> +++ b/tools/libxl/libxl_utils.c >> @@ -664,7 +664,8 @@ int libxl_nodemap_to_cpumap(libxl_ctx *ctx, >> libxl_bitmap_set(cpumap, i); >> } >> out: >> - libxl_cputopology_list_free(tinfo, nr_cpus); >> + if (tinfo != NULL) >> + libxl_cputopology_list_free(tinfo, nr_cpus); >> return rc; >> } > I was about to reply exactly as in the other e-mail (the reply to > 12/28), but in this case I think there is some small rom for issues, > since nr_cpus is not initialized.Right.> > In fact, libxl_nodemap_to_cpumap() looks like this: > > 648 it libxl_nodemap_to_cpumap(libxl_ctx *ctx, > 649 const libxl_bitmap *nodemap, > 650 libxl_bitmap *cpumap) > 651 { > 652 libxl_cputopology *tinfo = NULL; > 653 int nr_cpus, i, rc = 0; > 654 > 655 tinfo = libxl_get_cpu_topology(ctx, &nr_cpus); > 656 if (tinfo == NULL) { > 657 rc = ERROR_FAIL; > 658 goto out; > 659 } > 660 > 661 libxl_bitmap_set_none(cpumap); > 662 for (i = 0; i < nr_cpus; i++) { > 663 if (libxl_bitmap_test(nodemap, tinfo[i].node)) > 664 libxl_bitmap_set(cpumap, i); > 665 } > 666 out: > 667 libxl_cputopology_list_free(tinfo, nr_cpus); > 668 return rc; > 669 } > > Still, I think I''d like this fixed by doing something like this: > > 653 int nr_cpus = 0, i, rc = 0; > > rather than with what this patch does, for the same reasons already > explained in the other e-mails. > > I''m up for sending such patch myself, but that will be in a while, as > right now I''m away from my workstation. > > Is that fine with you?Sure, or I can do it myself soon (ie. as a reply to this, or in the next batch of patches). There''s no rush (AFAIK ;) ) - Matthew> >> >> @@ -685,7 +686,8 @@ int libxl_cpumap_to_nodemap(libxl_ctx *ctx, >> libxl_for_each_set_bit(i, *cpumap) >> libxl_bitmap_set(nodemap, tinfo[i].node); >> out: >> - libxl_cputopology_list_free(tinfo, nr_cpus); >> + if (tinfo != NULL) >> + libxl_cputopology_list_free(tinfo, nr_cpus); >> return rc; >> } >> > And the same applies here, of course. > > Is that fine with you guys? > > Regards, > Dario > > -- > <<This happens because I choose it to happen!>> (Raistlin Majere) > ----------------------------------------------------------------- > Dario Faggioli, Ph.D, http://about.me/dario.faggioli > Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK) >
Matthew Daley
2013-Sep-18 08:58 UTC
Re: [PATCH 10/28] libxl: only free cpupoolinfo if necessary in libxl_list_cpupool
On Wed, Sep 18, 2013 at 8:06 PM, Dario Faggioli <dario.faggioli@citrix.com> wrote:> I''m less confident about libxl_cpupoolinfo_list_* than about topology > and numa info handling via libxl, but I suspect this is something > similar to what I just said in my reply to 12/28 ("libxl: only free > cputopology if it was allocated in libxl__get_numa_candidate"). > > That is to say, this does not look necessary to me, as > libxl_cpupoolinfo_list_free() seems to cope well with a non-allocated > prt, and making it like this deviates from the usual libxl exit > pattern/error handling style. > > Thoughts?I think you''re right, again ;) Please ignore this patch, too. - Matthew
Dario Faggioli
2013-Sep-18 08:59 UTC
Re: [PATCH 13/28] libxl: only free cputopology if it was allocated in libxl_{cpu, node}map_to_{node, cpu}map
On mer, 2013-09-18 at 20:50 +1200, Matthew Daley wrote:> On Wed, Sep 18, 2013 at 8:18 PM, Dario Faggioli > <dario.faggioli@citrix.com> wrote: > > On mer, 2013-09-18 at 15:37 +1200, Matthew Daley wrote: > >> Coverity-ID: 1055294 > >> Coverity-ID: 1055295 > >> > > Ok, forgive my coverity ignorance and sorry if I should know this > > already, but I do I use these IDs to get some useful information? > > You can if you have access to the scan results. I''ve just been adding > them to the commit messages as that''s what I''ve seen done in other > projects (and other people''s Coverity-related Xen patches here). It > also helps in situations like these. >Ok, fine, just curious, but no big deal at all. Basically, I wanted to put some of those ''detailed info'' in the changelog of the patch we''re talking about below, but again, no big deal, I''ll (if I get there first) just put the IDs there.> > Still, I think I''d like this fixed by doing something like this: > > > > 653 int nr_cpus = 0, i, rc = 0; > > > > rather than with what this patch does, for the same reasons already > > explained in the other e-mails. > > > > I''m up for sending such patch myself, but that will be in a while, as > > right now I''m away from my workstation. > > > > Is that fine with you? > > Sure, or I can do it myself soon (ie. as a reply to this, or in the > next batch of patches). There''s no rush (AFAIK ;) ) >Indeed. Whoever gets to it first will do and send the patch. :-) If it is you, please, do Cc me (I''ll do the same). Thanks and Regards, Dario -- <<This happens because I choose it to happen!>> (Raistlin Majere) ----------------------------------------------------------------- Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK) _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Dario Faggioli
2013-Sep-18 09:10 UTC
Re: [PATCH 12/28] libxl: only free cputopology if it was allocated in libxl__get_numa_candidate
On mer, 2013-09-18 at 20:42 +1200, Matthew Daley wrote:> On Wed, Sep 18, 2013 at 7:57 PM, Dario Faggioli > > > Also, if something like that would be necessary, why doing it only for > > tinfo and not for ninfo as well? I''m not that into coverity, but I don''t > > think I see why it treats the two of them differently... :-O > > Not sure. Probably because if ninfo == NULL, we return directly out of > the function instead of goto-ing the cleanup. >Right, which, now that I''ve seen it, calls for a patch making it go through cleanup too! :-P Anyway, for libxl_cputopology_list_free() (and also for libxl_numainfo_list_free() if I really decide to patch it to make it more "libxl style"), especially considering that something similar to this happens in other places (as we''re seeing in 13/28), is there anything we can do for preventing this to come up again in future scans? Regards, Dario -- <<This happens because I choose it to happen!>> (Raistlin Majere) ----------------------------------------------------------------- Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK) _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Matthew Daley
2013-Sep-18 09:30 UTC
Re: [PATCH 12/28] libxl: only free cputopology if it was allocated in libxl__get_numa_candidate
On Wed, Sep 18, 2013 at 9:10 PM, Dario Faggioli <dario.faggioli@citrix.com> wrote:> On mer, 2013-09-18 at 20:42 +1200, Matthew Daley wrote: >> On Wed, Sep 18, 2013 at 7:57 PM, Dario Faggioli >> >> > Also, if something like that would be necessary, why doing it only for >> > tinfo and not for ninfo as well? I''m not that into coverity, but I don''t >> > think I see why it treats the two of them differently... :-O >> >> Not sure. Probably because if ninfo == NULL, we return directly out of >> the function instead of goto-ing the cleanup. >> > Right, which, now that I''ve seen it, calls for a patch making it go > through cleanup too! :-P > > Anyway, for libxl_cputopology_list_free() (and also for > libxl_numainfo_list_free() if I really decide to patch it to make it > more "libxl style"), especially considering that something similar to > this happens in other places (as we''re seeing in 13/28), is there > anything we can do for preventing this to come up again in future scans?Simply marking the Defects in Coverity as "False Positive" (which I just did before) prevents them from bugging us in the future. A quick grep shows that the functions are only called in ~11 spots AFAICT; that''s still small enough to sensibly allow manual overriding if any of the other invocations are complained about. However, I can''t see any others from a quick look (I haven''t figured out a way, if there is one, to go from a function *out* to its invocations which are tagged, unfortunately.) - Matthew
Samuel Thibault
2013-Sep-18 11:46 UTC
Re: [PATCH 21/28] mini-os: fix nodename generation in init_netfront
Matthew Daley, le Wed 18 Sep 2013 15:37:57 +1200, a écrit :> Using strlen here makes no sense. > > Coverity-ID: 1056053 > Signed-off-by: Matthew Daley <mattjd@gmail.com>Acked-by: Samuel Thibault <samuel.thibault@ens-lyon.org>> --- > extras/mini-os/netfront.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/extras/mini-os/netfront.c b/extras/mini-os/netfront.c > index 4e087a5..a999985 100644 > --- a/extras/mini-os/netfront.c > +++ b/extras/mini-os/netfront.c > @@ -309,8 +309,10 @@ struct netfront_dev *init_netfront(char *_nodename, void (*thenetif_rx)(unsigned > > if (!_nodename) > snprintf(nodename, sizeof(nodename), "device/vif/%d", netfrontends); > - else > - strncpy(nodename, _nodename, strlen(nodename)); > + else { > + strncpy(nodename, _nodename, sizeof(nodename) - 1); > + nodename[sizeof(nodename) - 1] = 0; > + } > netfrontends++; > > if (!thenetif_rx) > -- > 1.7.10.4 >
Samuel Thibault
2013-Sep-18 11:47 UTC
Re: [PATCH 22/28] mini-os: fix various memory leaks in blkfront
Matthew Daley, le Wed 18 Sep 2013 15:37:58 +1200, a écrit :> Coverity-ID: 1055814 > Coverity-ID: 1055815 > Signed-off-by: Matthew Daley <mattjd@gmail.com>Acked-by: Samuel Thibault <samuel.thibault@ens-lyon.org>> --- > extras/mini-os/blkfront.c | 11 +++++++---- > 1 file changed, 7 insertions(+), 4 deletions(-) > > diff --git a/extras/mini-os/blkfront.c b/extras/mini-os/blkfront.c > index ddcf665..aead6bd 100644 > --- a/extras/mini-os/blkfront.c > +++ b/extras/mini-os/blkfront.c > @@ -250,7 +250,7 @@ error: > > void shutdown_blkfront(struct blkfront_dev *dev) > { > - char* err = NULL; > + char* err = NULL, *err2; > XenbusState state; > > char path[strlen(dev->backend) + strlen("/state") + 1]; > @@ -295,12 +295,15 @@ void shutdown_blkfront(struct blkfront_dev *dev) > > close: > if (err) free(err); > - xenbus_unwatch_path_token(XBT_NIL, path, path); > + err2 = xenbus_unwatch_path_token(XBT_NIL, path, path); > + if (err2) free(err2); > > snprintf(nodename, sizeof(nodename), "%s/ring-ref", dev->nodename); > - xenbus_rm(XBT_NIL, nodename); > + err2 = xenbus_rm(XBT_NIL, nodename); > + if (err2) free(err2); > snprintf(nodename, sizeof(nodename), "%s/event-channel", dev->nodename); > - xenbus_rm(XBT_NIL, nodename); > + err2 = xenbus_rm(XBT_NIL, nodename); > + if (err2) free(err2); > > if (!err) > free_blkfront(dev); > -- > 1.7.10.4 >-- Samuel <b> il faut combien de chevaux pour tirer une doloréan à 88 morph ? ***b vient de remarque que 88 mph c''est 142 km/h <y> aaaaah <y> c''est pour ça qu''ils limitent à 130 km/h sur les autoroutes <y> c''est pour éviter que les gens voyagent dans le temps <b> probablement
Samuel Thibault
2013-Sep-18 11:50 UTC
Re: [PATCH 23/28] mini-os: fix various memory leaks in {fb, kbd}front
Matthew Daley, le Wed 18 Sep 2013 15:37:59 +1200, a écrit :> Coverity-ID: 1055819-1055826 > Signed-off-by: Matthew Daley <mattjd@gmail.com>Acked-by: Samuel Thibault <samuel.thibault@ens-lyon.org>> --- > extras/mini-os/fbfront.c | 48 ++++++++++++++++++++++++++++++---------------- > 1 file changed, 31 insertions(+), 17 deletions(-) > > diff --git a/extras/mini-os/fbfront.c b/extras/mini-os/fbfront.c > index 3d0b8f5..a276193 100644 > --- a/extras/mini-os/fbfront.c > +++ b/extras/mini-os/fbfront.c > @@ -171,7 +171,8 @@ done: > err = xenbus_wait_for_state_change(path, &state, &dev->events); > if (state != XenbusStateConnected) { > printk("backend not available, state=%d\n", state); > - xenbus_unwatch_path_token(XBT_NIL, path, path); > + if (err) free(err); > + err = xenbus_unwatch_path_token(XBT_NIL, path, path); > goto error; > } > > @@ -181,7 +182,8 @@ done: > if((err = xenbus_switch_state(XBT_NIL, frontpath, XenbusStateConnected)) > != NULL) { > printk("error switching state: %s\n", err); > - xenbus_unwatch_path_token(XBT_NIL, path, path); > + free(err); > + err = xenbus_unwatch_path_token(XBT_NIL, path, path); > goto error; > } > } > @@ -236,7 +238,7 @@ int kbdfront_receive(struct kbdfront_dev *dev, union xenkbd_in_event *buf, int n > > void shutdown_kbdfront(struct kbdfront_dev *dev) > { > - char* err = NULL; > + char* err = NULL, *err2; > XenbusState state; > > char path[strlen(dev->backend) + strlen("/state") + 1]; > @@ -278,14 +280,18 @@ void shutdown_kbdfront(struct kbdfront_dev *dev) > > close_kbdfront: > if (err) free(err); > - xenbus_unwatch_path_token(XBT_NIL, path, path); > + err2 = xenbus_unwatch_path_token(XBT_NIL, path, path); > + if (err2) free(err2); > > snprintf(nodename, sizeof(nodename), "%s/page-ref", dev->nodename); > - xenbus_rm(XBT_NIL, nodename); > + err2 = xenbus_rm(XBT_NIL, nodename); > + if (err2) free(err2); > snprintf(nodename, sizeof(nodename), "%s/event-channel", dev->nodename); > - xenbus_rm(XBT_NIL, nodename); > + err2 = xenbus_rm(XBT_NIL, nodename); > + if (err2) free(err2); > snprintf(nodename, sizeof(nodename), "%s/request-abs-pointer", dev->nodename); > - xenbus_rm(XBT_NIL, nodename); > + err2 = xenbus_rm(XBT_NIL, nodename); > + if (err2) free(err2); > > if (!err) > free_kbdfront(dev); > @@ -405,7 +411,7 @@ struct fbfront_dev *init_fbfront(char *_nodename, unsigned long *mfns, int width > char* message=NULL; > struct xenfb_page *s; > int retry=0; > - char* msg; > + char* msg=NULL; > int i, j; > struct fbfront_dev *dev; > int max_pd; > @@ -532,7 +538,8 @@ done: > err = xenbus_wait_for_state_change(path, &state, &dev->events); > if (state != XenbusStateConnected) { > printk("backend not available, state=%d\n", state); > - xenbus_unwatch_path_token(XBT_NIL, path, path); > + if (err) free(err); > + err = xenbus_unwatch_path_token(XBT_NIL, path, path); > goto error; > } > > @@ -545,7 +552,8 @@ done: > if ((err = xenbus_switch_state(XBT_NIL, frontpath, XenbusStateConnected)) > != NULL) { > printk("error switching state: %s\n", err); > - xenbus_unwatch_path_token(XBT_NIL, path, path); > + free(err); > + err = xenbus_unwatch_path_token(XBT_NIL, path, path); > goto error; > } > } > @@ -556,6 +564,7 @@ done: > return dev; > > error: > + free(msg); > free(err); > free_fbfront(dev); > return NULL; > @@ -627,7 +636,7 @@ void fbfront_resize(struct fbfront_dev *dev, int width, int height, int stride, > > void shutdown_fbfront(struct fbfront_dev *dev) > { > - char* err = NULL; > + char* err = NULL, *err2; > XenbusState state; > > char path[strlen(dev->backend) + strlen("/state") + 1]; > @@ -654,7 +663,7 @@ void shutdown_fbfront(struct fbfront_dev *dev) > } > state = xenbus_read_integer(path); > if (state < XenbusStateClosed) { > - xenbus_wait_for_state_change(path, &state, &dev->events); > + err = xenbus_wait_for_state_change(path, &state, &dev->events); > if (err) free(err); > } > > @@ -669,16 +678,21 @@ void shutdown_fbfront(struct fbfront_dev *dev) > > close_fbfront: > if (err) free(err); > - xenbus_unwatch_path_token(XBT_NIL, path, path); > + err2 = xenbus_unwatch_path_token(XBT_NIL, path, path); > + if (err2) free(err2); > > snprintf(nodename, sizeof(nodename), "%s/page-ref", dev->nodename); > - xenbus_rm(XBT_NIL, nodename); > + err2 = xenbus_rm(XBT_NIL, nodename); > + if (err2) free(err2); > snprintf(nodename, sizeof(nodename), "%s/event-channel", dev->nodename); > - xenbus_rm(XBT_NIL, nodename); > + err2 = xenbus_rm(XBT_NIL, nodename); > + if (err2) free(err2); > snprintf(nodename, sizeof(nodename), "%s/protocol", dev->nodename); > - xenbus_rm(XBT_NIL, nodename); > + err2 = xenbus_rm(XBT_NIL, nodename); > + if (err2) free(err2); > snprintf(nodename, sizeof(nodename), "%s/feature-update", dev->nodename); > - xenbus_rm(XBT_NIL, nodename); > + err2 = xenbus_rm(XBT_NIL, nodename); > + if (err2) free(err2); > > if (!err) > free_fbfront(dev); > -- > 1.7.10.4 >-- Samuel I am the "ILOVEGNU" signature virus. Just copy me to your signature. This email was infected under the terms of the GNU General Public License.
Samuel Thibault
2013-Sep-18 11:50 UTC
Re: [PATCH 24/28] mini-os: fix various memory leaks in netfront
Matthew Daley, le Wed 18 Sep 2013 15:38:00 +1200, a écrit :> Coverity-ID: 1055832 > Coverity-ID: 1055833 > Signed-off-by: Matthew Daley <mattjd@gmail.com>Acked-by: Samuel Thibault <samuel.thibault@ens-lyon.org>> --- > extras/mini-os/netfront.c | 17 +++++++++++------ > 1 file changed, 11 insertions(+), 6 deletions(-) > > diff --git a/extras/mini-os/netfront.c b/extras/mini-os/netfront.c > index a999985..3a5be64 100644 > --- a/extras/mini-os/netfront.c > +++ b/extras/mini-os/netfront.c > @@ -506,7 +506,7 @@ int netfront_tap_open(char *nodename) { > > void shutdown_netfront(struct netfront_dev *dev) > { > - char* err = NULL; > + char* err = NULL, *err2; > XenbusState state; > > char path[strlen(dev->backend) + strlen("/state") + 1]; > @@ -549,16 +549,21 @@ void shutdown_netfront(struct netfront_dev *dev) > > close: > if (err) free(err); > - xenbus_unwatch_path_token(XBT_NIL, path, path); > + err2 = xenbus_unwatch_path_token(XBT_NIL, path, path); > + if (err2) free(err2); > > snprintf(nodename, sizeof(nodename), "%s/tx-ring-ref", dev->nodename); > - xenbus_rm(XBT_NIL, nodename); > + err2 = xenbus_rm(XBT_NIL, nodename); > + if (err2) free(err2); > snprintf(nodename, sizeof(nodename), "%s/rx-ring-ref", dev->nodename); > - xenbus_rm(XBT_NIL, nodename); > + err2 = xenbus_rm(XBT_NIL, nodename); > + if (err2) free(err2); > snprintf(nodename, sizeof(nodename), "%s/event-channel", dev->nodename); > - xenbus_rm(XBT_NIL, nodename); > + err2 = xenbus_rm(XBT_NIL, nodename); > + if (err2) free(err2); > snprintf(nodename, sizeof(nodename), "%s/request-rx-copy", dev->nodename); > - xenbus_rm(XBT_NIL, nodename); > + err2 = xenbus_rm(XBT_NIL, nodename); > + if (err2) free(err2); > > if (!err) > free_netfront(dev); > -- > 1.7.10.4 >-- Samuel As usual, this being a 1.3.x release, I haven''t even compiled this kernel yet. So if it works, you should be doubly impressed. (Linus Torvalds, announcing kernel 1.3.3 on the linux-kernel mailing list.)
Samuel Thibault
2013-Sep-18 11:51 UTC
Re: [PATCH 25/28] mini-os: fix various memory leaks in pcifront
Matthew Daley, le Wed 18 Sep 2013 15:38:01 +1200, a écrit :> Coverity-ID: 1055834-1055840 > Signed-off-by: Matthew Daley <mattjd@gmail.com>Acked-by: Samuel Thibault <samuel.thibault@ens-lyon.org>> --- > extras/mini-os/pcifront.c | 29 +++++++++++++++++++---------- > 1 file changed, 19 insertions(+), 10 deletions(-) > > diff --git a/extras/mini-os/pcifront.c b/extras/mini-os/pcifront.c > index cdf9c9b..7838021 100644 > --- a/extras/mini-os/pcifront.c > +++ b/extras/mini-os/pcifront.c > @@ -122,11 +122,12 @@ void pcifront_watches(void *opaque) > } else if (state == XenbusStateClosing) > break; > } > - if (err) > + if (err) { > printk("pcifront_watches: done waiting err=%s\n", err); > - else > + free(err); > + } else > printk("pcifront_watches: done waiting\n"); > - xenbus_unwatch_path_token(XBT_NIL, be_state, be_state); > + err = xenbus_unwatch_path_token(XBT_NIL, be_state, be_state); > shutdown_pcifront(pcidev); > free(be_state); > free(be_path); > @@ -143,7 +144,7 @@ struct pcifront_dev *init_pcifront(char *_nodename) > char* err; > char* message=NULL; > int retry=0; > - char* msg; > + char* msg = NULL; > char* nodename = _nodename ? _nodename : "device/pci/0"; > int dom; > > @@ -250,7 +251,8 @@ done: > err = xenbus_wait_for_state_change(path, &state, &dev->events); > if (state != XenbusStateConnected) { > printk("backend not avalable, state=%d\n", state); > - xenbus_unwatch_path_token(XBT_NIL, path, path); > + if (err) free(err); > + err = xenbus_unwatch_path_token(XBT_NIL, path, path); > goto error; > } > > @@ -258,7 +260,8 @@ done: > if ((err = xenbus_switch_state(XBT_NIL, frontpath, XenbusStateConnected)) > != NULL) { > printk("error switching state %s\n", err); > - xenbus_unwatch_path_token(XBT_NIL, path, path); > + free(err); > + err = xenbus_unwatch_path_token(XBT_NIL, path, path); > goto error; > } > } > @@ -272,6 +275,7 @@ done: > return dev; > > error: > + free(msg); > free(err); > free_pcifront(dev); > return NULL; > @@ -301,6 +305,7 @@ void pcifront_scan(struct pcifront_dev *dev, void (*func)(unsigned int domain, u > msg = xenbus_read(XBT_NIL, path, &s); > if (msg) { > printk("Error %s when reading the PCI root name at %s\n", msg, path); > + free(msg); > continue; > } > > @@ -319,7 +324,7 @@ void pcifront_scan(struct pcifront_dev *dev, void (*func)(unsigned int domain, u > > void shutdown_pcifront(struct pcifront_dev *dev) > { > - char* err = NULL; > + char* err = NULL, *err2; > XenbusState state; > > char path[strlen(dev->backend) + strlen("/state") + 1]; > @@ -361,12 +366,15 @@ void shutdown_pcifront(struct pcifront_dev *dev) > > close_pcifront: > if (err) free(err); > - xenbus_unwatch_path_token(XBT_NIL, path, path); > + err2 = xenbus_unwatch_path_token(XBT_NIL, path, path); > + if (err2) free(err2); > > snprintf(nodename, sizeof(nodename), "%s/info-ref", dev->nodename); > - xenbus_rm(XBT_NIL, nodename); > + err2 = xenbus_rm(XBT_NIL, nodename); > + if (err2) free(err2); > snprintf(nodename, sizeof(nodename), "%s/event-channel", dev->nodename); > - xenbus_rm(XBT_NIL, nodename); > + err2 = xenbus_rm(XBT_NIL, nodename); > + if (err2) free(err2); > > if (!err) > free_pcifront(dev); > @@ -397,6 +405,7 @@ int pcifront_physical_to_virtual (struct pcifront_dev *dev, > msg = xenbus_read(XBT_NIL, path, &s); > if (msg) { > printk("Error %s when reading the PCI root name at %s\n", msg, path); > + free(msg); > continue; > } > > -- > 1.7.10.4 >-- Samuel <c> ya(ka|ma|to)* ca existe une fois sur 2 au japon, c''est facile ;-) -+- #ens-mim au japon -+-
Samuel Thibault
2013-Sep-18 11:53 UTC
Re: [PATCH 26/28] mini-os: fix various memory leaks in consfront
Matthew Daley, le Wed 18 Sep 2013 15:38:02 +1200, a écrit :> Coverity-ID: 1055816 > Coverity-ID: 1055817 > Coverity-ID: 1055818 > Signed-off-by: Matthew Daley <mattjd@gmail.com>Acked-by: Samuel Thibault <samuel.thibault@ens-lyon.org>> --- > extras/mini-os/console/xenbus.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/extras/mini-os/console/xenbus.c b/extras/mini-os/console/xenbus.c > index 4194982..b317114 100644 > --- a/extras/mini-os/console/xenbus.c > +++ b/extras/mini-os/console/xenbus.c > @@ -42,7 +42,8 @@ void free_consfront(struct consfront_dev *dev) > > close: > if (err) free(err); > - xenbus_unwatch_path_token(XBT_NIL, path, path); > + err = xenbus_unwatch_path_token(XBT_NIL, path, path); > + if (err) free(err); > > mask_evtchn(dev->evtchn); > unbind_evtchn(dev->evtchn); > @@ -58,7 +59,7 @@ close: > struct consfront_dev *init_consfront(char *_nodename) > { > xenbus_transaction_t xbt; > - char* err; > + char* err = NULL; > char* message=NULL; > int retry=0; > char* msg = NULL; > @@ -87,7 +88,7 @@ struct consfront_dev *init_consfront(char *_nodename) > > snprintf(path, sizeof(path), "%s/backend-id", nodename); > if ((res = xenbus_read_integer(path)) < 0) > - return NULL; > + goto error; > else > dev->dom = res; > evtchn_alloc_unbound(dev->dom, console_handle_input, dev, &dev->evtchn); > @@ -170,7 +171,7 @@ done: > msg = xenbus_wait_for_state_change(path, &state, &dev->events); > if (msg != NULL || state != XenbusStateConnected) { > printk("backend not available, state=%d\n", state); > - xenbus_unwatch_path_token(XBT_NIL, path, path); > + err = xenbus_unwatch_path_token(XBT_NIL, path, path); > goto error; > } > } > -- > 1.7.10.4 >-- Samuel Now I know someone out there is going to claim, "Well then, UNIX is intuitive, because you only need to learn 5000 commands, and then everything else follows from that! Har har har!" (Andy Bates in comp.os.linux.misc, on "intuitive interfaces", slightly defending Macs.)
Samuel Thibault
2013-Sep-18 11:54 UTC
Re: [PATCH 27/28] mini-os: fix various memory leaks in various locations
Matthew Daley, le Wed 18 Sep 2013 15:38:03 +1200, a écrit :> Coverity-ID: 1055827 > Coverity-ID: 1055828 > Coverity-ID: 1055829 > Coverity-ID: 1055830 > Coverity-ID: 1055831 > Signed-off-by: Matthew Daley <mattjd@gmail.com>Acked-by: Samuel Thibault <samuel.thibault@ens-lyon.org>> --- > extras/mini-os/kernel.c | 7 +++++-- > extras/mini-os/lib/xs.c | 2 ++ > 2 files changed, 7 insertions(+), 2 deletions(-) > > diff --git a/extras/mini-os/kernel.c b/extras/mini-os/kernel.c > index e9402b9..24fa25c 100644 > --- a/extras/mini-os/kernel.c > +++ b/extras/mini-os/kernel.c > @@ -84,8 +84,10 @@ static void shutdown_thread(void *p) > free(err); > xenbus_wait_for_watch(&events); > } > - xenbus_unwatch_path_token(XBT_NIL, path, token); > - xenbus_write(XBT_NIL, path, ""); > + err = xenbus_unwatch_path_token(XBT_NIL, path, token); > + if (err) free(err); > + err = xenbus_write(XBT_NIL, path, ""); > + if (err) free(err); > printk("Shutting down (%s)\n", shutdown); > > if (!strcmp(shutdown, "poweroff")) > @@ -96,6 +98,7 @@ static void shutdown_thread(void *p) > /* Unknown */ > shutdown_reason = SHUTDOWN_crash; > app_shutdown(shutdown_reason); > + free(shutdown); > } > #endif > > diff --git a/extras/mini-os/lib/xs.c b/extras/mini-os/lib/xs.c > index c603d17..324bd05 100644 > --- a/extras/mini-os/lib/xs.c > +++ b/extras/mini-os/lib/xs.c > @@ -144,6 +144,7 @@ char **xs_directory(struct xs_handle *h, xs_transaction_t t, > msg = xenbus_ls(t, path, &res); > if (msg) { > printk("xs_directory(%s): %s\n", path, msg); > + free(msg); > return NULL; > } > > @@ -163,6 +164,7 @@ char **xs_directory(struct xs_handle *h, xs_transaction_t t, > } > > *num = n; > + free(res); > return entries; > } > > -- > 1.7.10.4 >-- Samuel ***e trouve un .xls ***e passe un moment à se demander quelle version de xml c''est ça, le .xls e: donc j''ai fait un file.... -+- #sos - on n''a pas forcément les mêmes références que tout le monde -+-
Andrew Cooper
2013-Sep-18 12:29 UTC
Re: [PATCH 11/28] libxl: only put poller if already gotten in libxl_event_wait
On 18/09/2013 04:37, Matthew Daley wrote:> Coverity-ID: 1055292 > Signed-off-by: Matthew Daley <mattjd@gmail.com> > --- > tools/libxl/libxl_event.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tools/libxl/libxl_event.c b/tools/libxl/libxl_event.c > index e42b371..fd2f280 100644 > --- a/tools/libxl/libxl_event.c > +++ b/tools/libxl/libxl_event.c > @@ -1476,7 +1476,7 @@ int libxl_event_wait(libxl_ctx *ctx, libxl_event **event_r, > } > > out: > - libxl__poller_put(ctx, poller); > + if (poller) libxl__poller_put(ctx, poller);Coding style would require the code to look like this: if ( poller ) libxl__poller_put(ctx, poller); ~Andrew> > CTX_UNLOCK; > EGC_FREE;
Andrew Cooper
2013-Sep-18 12:39 UTC
Re: [PATCH 23/28] mini-os: fix various memory leaks in {fb, kbd}front
On 18/09/2013 04:37, Matthew Daley wrote:> Coverity-ID: 1055819-1055826 > Signed-off-by: Matthew Daley <mattjd@gmail.com> > --- > extras/mini-os/fbfront.c | 48 ++++++++++++++++++++++++++++++---------------- > 1 file changed, 31 insertions(+), 17 deletions(-) > > diff --git a/extras/mini-os/fbfront.c b/extras/mini-os/fbfront.c > index 3d0b8f5..a276193 100644 > --- a/extras/mini-os/fbfront.c > +++ b/extras/mini-os/fbfront.c > @@ -171,7 +171,8 @@ done: > err = xenbus_wait_for_state_change(path, &state, &dev->events); > if (state != XenbusStateConnected) { > printk("backend not available, state=%d\n", state); > - xenbus_unwatch_path_token(XBT_NIL, path, path); > + if (err) free(err); > + err = xenbus_unwatch_path_token(XBT_NIL, path, path); > goto error; > } > > @@ -181,7 +182,8 @@ done: > if((err = xenbus_switch_state(XBT_NIL, frontpath, XenbusStateConnected)) > != NULL) { > printk("error switching state: %s\n", err); > - xenbus_unwatch_path_token(XBT_NIL, path, path); > + free(err); > + err = xenbus_unwatch_path_token(XBT_NIL, path, path); > goto error; > } > } > @@ -236,7 +238,7 @@ int kbdfront_receive(struct kbdfront_dev *dev, union xenkbd_in_event *buf, int n > > void shutdown_kbdfront(struct kbdfront_dev *dev) > { > - char* err = NULL; > + char* err = NULL, *err2; > XenbusState state; > > char path[strlen(dev->backend) + strlen("/state") + 1]; > @@ -278,14 +280,18 @@ void shutdown_kbdfront(struct kbdfront_dev *dev) > > close_kbdfront: > if (err) free(err); > - xenbus_unwatch_path_token(XBT_NIL, path, path); > + err2 = xenbus_unwatch_path_token(XBT_NIL, path, path); > + if (err2) free(err2); > > snprintf(nodename, sizeof(nodename), "%s/page-ref", dev->nodename); > - xenbus_rm(XBT_NIL, nodename); > + err2 = xenbus_rm(XBT_NIL, nodename); > + if (err2) free(err2); > snprintf(nodename, sizeof(nodename), "%s/event-channel", dev->nodename); > - xenbus_rm(XBT_NIL, nodename); > + err2 = xenbus_rm(XBT_NIL, nodename); > + if (err2) free(err2); > snprintf(nodename, sizeof(nodename), "%s/request-abs-pointer", dev->nodename); > - xenbus_rm(XBT_NIL, nodename); > + err2 = xenbus_rm(XBT_NIL, nodename); > + if (err2) free(err2); > > if (!err) > free_kbdfront(dev); > @@ -405,7 +411,7 @@ struct fbfront_dev *init_fbfront(char *_nodename, unsigned long *mfns, int width > char* message=NULL; > struct xenfb_page *s; > int retry=0; > - char* msg; > + char* msg=NULL; > int i, j; > struct fbfront_dev *dev; > int max_pd; > @@ -532,7 +538,8 @@ done: > err = xenbus_wait_for_state_change(path, &state, &dev->events); > if (state != XenbusStateConnected) { > printk("backend not available, state=%d\n", state); > - xenbus_unwatch_path_token(XBT_NIL, path, path); > + if (err) free(err);free() is able to deal with being passed a NULL pointer. Therefore, the "if (err)" is not needed. (And indeed, you dont use it in the next hunk) ~Andrew> + err = xenbus_unwatch_path_token(XBT_NIL, path, path); > goto error; > } > > @@ -545,7 +552,8 @@ done: > if ((err = xenbus_switch_state(XBT_NIL, frontpath, XenbusStateConnected)) > != NULL) { > printk("error switching state: %s\n", err); > - xenbus_unwatch_path_token(XBT_NIL, path, path); > + free(err); > + err = xenbus_unwatch_path_token(XBT_NIL, path, path); > goto error; > } > } > @@ -556,6 +564,7 @@ done: > return dev; > > error: > + free(msg); > free(err); > free_fbfront(dev); > return NULL; > @@ -627,7 +636,7 @@ void fbfront_resize(struct fbfront_dev *dev, int width, int height, int stride, > > void shutdown_fbfront(struct fbfront_dev *dev) > { > - char* err = NULL; > + char* err = NULL, *err2; > XenbusState state; > > char path[strlen(dev->backend) + strlen("/state") + 1]; > @@ -654,7 +663,7 @@ void shutdown_fbfront(struct fbfront_dev *dev) > } > state = xenbus_read_integer(path); > if (state < XenbusStateClosed) { > - xenbus_wait_for_state_change(path, &state, &dev->events); > + err = xenbus_wait_for_state_change(path, &state, &dev->events); > if (err) free(err); > } > > @@ -669,16 +678,21 @@ void shutdown_fbfront(struct fbfront_dev *dev) > > close_fbfront: > if (err) free(err); > - xenbus_unwatch_path_token(XBT_NIL, path, path); > + err2 = xenbus_unwatch_path_token(XBT_NIL, path, path); > + if (err2) free(err2); > > snprintf(nodename, sizeof(nodename), "%s/page-ref", dev->nodename); > - xenbus_rm(XBT_NIL, nodename); > + err2 = xenbus_rm(XBT_NIL, nodename); > + if (err2) free(err2); > snprintf(nodename, sizeof(nodename), "%s/event-channel", dev->nodename); > - xenbus_rm(XBT_NIL, nodename); > + err2 = xenbus_rm(XBT_NIL, nodename); > + if (err2) free(err2); > snprintf(nodename, sizeof(nodename), "%s/protocol", dev->nodename); > - xenbus_rm(XBT_NIL, nodename); > + err2 = xenbus_rm(XBT_NIL, nodename); > + if (err2) free(err2); > snprintf(nodename, sizeof(nodename), "%s/feature-update", dev->nodename); > - xenbus_rm(XBT_NIL, nodename); > + err2 = xenbus_rm(XBT_NIL, nodename); > + if (err2) free(err2); > > if (!err) > free_fbfront(dev);
Dario Faggioli
2013-Sep-18 12:54 UTC
Re: [PATCH 12/28] libxl: only free cputopology if it was allocated in libxl__get_numa_candidate
On mer, 2013-09-18 at 21:30 +1200, Matthew Daley wrote:> On Wed, Sep 18, 2013 at 9:10 PM, Dario Faggioli > > > Anyway, for libxl_cputopology_list_free() (and also for > > libxl_numainfo_list_free() if I really decide to patch it to make it > > more "libxl style"), especially considering that something similar to > > this happens in other places (as we''re seeing in 13/28), is there > > anything we can do for preventing this to come up again in future scans? > > Simply marking the Defects in Coverity as "False Positive" (which I > just did before) prevents them from bugging us in the future. >Oh, awesome!> A quick grep shows that the functions are only called in ~11 spots > AFAICT; that''s still small enough to sensibly allow manual overriding > if any of the other invocations are complained about. >Sounds good, thanks for explaining this. :-) Regards, Dario -- <<This happens because I choose it to happen!>> (Raistlin Majere) ----------------------------------------------------------------- Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK) _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Matthew Daley
2013-Sep-18 13:01 UTC
Re: [PATCH 23/28] mini-os: fix various memory leaks in {fb, kbd}front
On Thu, Sep 19, 2013 at 12:39 AM, Andrew Cooper <andrew.cooper3@citrix.com> wrote:> On 18/09/2013 04:37, Matthew Daley wrote: >> @@ -532,7 +538,8 @@ done: >> err = xenbus_wait_for_state_change(path, &state, &dev->events); >> if (state != XenbusStateConnected) { >> printk("backend not available, state=%d\n", state); >> - xenbus_unwatch_path_token(XBT_NIL, path, path); >> + if (err) free(err); > > free() is able to deal with being passed a NULL pointer. Therefore, the > "if (err)" is not needed. (And indeed, you dont use it in the next hunk)Right. I was just following the existing style, where it does have the redundant if()s - I count at least 22 cases in the existing code. I guess I can respin these mini-os patches, or I can do just one to remove both the newly added and the old cases across all of mini-os at once after these have gone in. - Matthew
Matthew Daley
2013-Sep-18 13:06 UTC
Re: [PATCH 11/28] libxl: only put poller if already gotten in libxl_event_wait
On Thu, Sep 19, 2013 at 12:29 AM, Andrew Cooper <andrew.cooper3@citrix.com> wrote:> On 18/09/2013 04:37, Matthew Daley wrote: >> Coverity-ID: 1055292 >> Signed-off-by: Matthew Daley <mattjd@gmail.com> >> --- >> tools/libxl/libxl_event.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/tools/libxl/libxl_event.c b/tools/libxl/libxl_event.c >> index e42b371..fd2f280 100644 >> --- a/tools/libxl/libxl_event.c >> +++ b/tools/libxl/libxl_event.c >> @@ -1476,7 +1476,7 @@ int libxl_event_wait(libxl_ctx *ctx, libxl_event **event_r, >> } >> >> out: >> - libxl__poller_put(ctx, poller); >> + if (poller) libxl__poller_put(ctx, poller); > > Coding style would require the code to look like this: > > if ( poller ) > libxl__poller_put(ctx, poller);Indeed. I was going off the check just after the call to libxl__poller get (and possibly confusing the style with Mini-OS''s), but I suppose this is only for rc/error-type checks. I''ll resend a fixed patch. - Matthew
Tim Deegan
2013-Sep-19 09:12 UTC
Re: [PATCH 01/28] gnttab: remove unused shared header lookup
At 15:37 +1200 on 18 Sep (1379518657), Matthew Daley wrote:> Coverity-ID: 1056171 > Signed-off-by: Matthew Daley <mattjd@gmail.com>Reviewed-by: Tim Deegan <tim@xen.org>> --- > xen/common/grant_table.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c > index eb50288..f42bc7a 100644 > --- a/xen/common/grant_table.c > +++ b/xen/common/grant_table.c > @@ -783,7 +783,6 @@ __gnttab_map_grant_ref( > spin_lock(&rgt->lock); > > act = &active_entry(rgt, op->ref); > - shah = shared_entry_header(rgt, op->ref); > > if ( op->flags & GNTMAP_device_map ) > act->pin -= (op->flags & GNTMAP_readonly) ? > -- > 1.7.10.4 > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel
Ian Campbell
2013-Sep-21 15:57 UTC
Re: [PATCH 00/28] Fixes for various minor Coverity issues, volume 2
On Wed, 2013-09-18 at 15:37 +1200, Matthew Daley wrote:> Tested by building and creating a few PV and HVM domains. There shouldn''t be > anything too complicated or controversial here. > > Matthew Daley (28): > mini-os: fix nodename generation in init_netfront > mini-os: fix various memory leaks in blkfront > mini-os: fix various memory leaks in {fb,kbd}front > mini-os: fix various memory leaks in netfront > mini-os: fix various memory leaks in pcifront > mini-os: fix various memory leaks in consfront > mini-os: fix various memory leaks in various locationsI applied this lot with Samuel''s acks.> gdbsx: clear sockaddr before using itI acked this and applied. I didn''t have a chance to look a the rest yet. Ian.
Ian Campbell
2013-Sep-25 11:39 UTC
Re: [PATCH 03/28] libxc: move munmap into the loop it''s needed in in change_pte
On Wed, 2013-09-18 at 15:37 +1200, Matthew Daley wrote: I think the commit message is wrong here, it''s not moving it inside a loop, it''s moving it inside a conditional. It''s already in the outer loop (over i) and putting it into the inner loop (over j) would be wrong I think.> Coverity-ID: 1055269 > Signed-off-by: Matthew Daley <mattjd@gmail.com> > --- > tools/libxc/xc_offline_page.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/tools/libxc/xc_offline_page.c b/tools/libxc/xc_offline_page.c > index fbb53f5..8195efb 100644 > --- a/tools/libxc/xc_offline_page.c > +++ b/tools/libxc/xc_offline_page.c > @@ -317,10 +317,10 @@ static int change_pte(xc_interface *xch, int domid, > goto failed; > } > } > - } > > - munmap(content, PAGE_SIZE); > - content = NULL; > + munmap(content, PAGE_SIZE); > + content = NULL; > + } > } > > if ( xc_flush_mmu_updates(xch, mmu) )
Ian Campbell
2013-Sep-25 12:04 UTC
Re: [PATCH 17/28] libxl: gettimeofday doesn''t return an errno on failure
On Wed, 2013-09-18 at 15:37 +1200, Matthew Daley wrote:
http://pubs.opengroup.org/onlinepubs/9699919799/functions/gettimeofday.html
agrees with this but the Linux manpages gettimeofday(2) disagrees:
gettimeofday() and settimeofday() return 0 for success, or -1 for
failure (in which case errno is set appropriately).
They may just have confused themselves by lumping get in with set, but
at least one error code (EFAULT) seems like it could apply to get too.
Since the spec says it cannot fail I think there''s no harm in reporting
errno if it does fail.
is the CHK_ERRNO macro correct?
#define CHK_ERRNO( call ) ({ \
int chk_errno = (call); \
if (chk_errno < 0) { \
fprintf(stderr,"xl: fatal error: %s:%d: %s: %s\n",
\
__FILE__,__LINE__, strerror(chk_errno), #call); \
exit(-ERROR_FAIL); \
} \
})
It seems to report the reutrn code and not errno, so it will always say
-1 won''t it?
I think the actual coverity error "chk_errno" is passed to a parameter
that cannot be negative." stems from this not the lack of errno, because
coverity seems to know that strerror cannot take a negative number.
> Coverity-ID: 1055570
> Signed-off-by: Matthew Daley <mattjd@gmail.com>
> ---
> tools/libxl/xl_cmdimpl.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index 642b130..7ec5d6a 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -3392,7 +3392,7 @@ static void migration_child_report(int recv_fd) {
>
> if (!xl_child_pid(child_migration)) return;
>
> - CHK_ERRNO( gettimeofday(&waituntil, 0) );
> + MUST( gettimeofday(&waituntil, 0) );
> waituntil.tv_sec += 2;
>
> for (;;) {
> @@ -3413,7 +3413,7 @@ static void migration_child_report(int recv_fd) {
> }
> assert(child == 0);
>
> - CHK_ERRNO( gettimeofday(&now, 0) );
> + MUST( gettimeofday(&now, 0) );
> if (timercmp(&now, &waituntil, >)) {
> fprintf(stderr, "migration child [%ld] not exiting, no
longer"
> " waiting (exit status will be
unreported)\n",
Ian Campbell
2013-Sep-25 12:57 UTC
Re: [PATCH 02/28] libxc: fix memory leak in load_p2m_frame_list error handling
On Wed, 2013-09-18 at 15:37 +1200, Matthew Daley wrote:> Coverity-ID: 1055885 > Signed-off-by: Matthew Daley <mattjd@gmail.com>Acked + applied. Thanks.> --- > tools/libxc/xc_domain_restore.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/tools/libxc/xc_domain_restore.c b/tools/libxc/xc_domain_restore.c > index b418963..939a76b 100644 > --- a/tools/libxc/xc_domain_restore.c > +++ b/tools/libxc/xc_domain_restore.c > @@ -368,6 +368,7 @@ static xen_pfn_t *load_p2m_frame_list( > (P2M_FL_ENTRIES - 1) * sizeof(xen_pfn_t)) ) > { > PERROR("read p2m_frame_list failed"); > + free(p2m_frame_list); > return NULL; > } >
Ian Campbell
2013-Sep-25 12:58 UTC
Re: [PATCH 04/28] libxl: fix libxl_string_list_length and its only caller
On Wed, 2013-09-18 at 15:37 +1200, Matthew Daley wrote:> The wrong amount of indirections were being taken in > libxl_string_list_length, and its only caller was miscounting the amount > of initial non-list arguments, seemingly since the initial commit > (599c784). > > This has been seen and reported in the wild (##xen): > < Trixboxer> Hi, any idea why would I get > < Trixboxer> xl: libxl_bootloader.c:42: bootloader_arg: Assertion `bl->nargs < bl->argsspace'' failed. > < Trixboxer> 4.2.2-23.el6 > > Coverity-ID: 1054954 > Signed-off-by: Matthew Daley <mattjd@gmail.com>acked + applied, thanks
Ian Campbell
2013-Sep-25 13:01 UTC
Re: [PATCH 05/28] libxl: fix dispose without init of disk in cd_insert
On Wed, 2013-09-18 at 15:37 +1200, Matthew Daley wrote:> Coverity-ID: 1056078 > Signed-off-by: Matthew Daley <mattjd@gmail.com>Acked-by + applied this: + [PATCH 06/28] libxl: fix leak of corename in handle_domain_death [PATCH 07/28] libxl: fix leak of config_data in main_cpupoolcreate [PATCH 08/28] libxl: fix leak of rune in main_remus [PATCH 09/28] libxl: fix out-of-memory check in parse_global_config [PATCH 14/28] libxl: only free console reader if it was allocated in main_dmesg [PATCH 15/28] libxl: fix typo in libxl__hotplug_nic error checking [PATCH 16/28] libxl: fix file open failure check in libxl__file_reference_map [PATCH 18/28] xenstored: handle unlikely failure better in ask_parents [PATCH 19/28] xenstored: fix faulty check for bad handle in domain_init [PATCH 20/28] xenstore: check socket path length before copying it I think the rest are either applied already or I or someone else has commented on it. Thanks, Ian.
Ian Campbell
2013-Sep-25 13:06 UTC
Re: [PATCH 18/28] xenstored: handle unlikely failure better in ask_parents
On Wed, 2013-09-18 at 15:37 +1200, Matthew Daley wrote:> Coverity-ID: 1055277 > Signed-off-by: Matthew Daley <mattjd@gmail.com> > --- > tools/xenstore/xenstored_core.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c > index a2cf2a6..0f8ba64 100644 > --- a/tools/xenstore/xenstored_core.c > +++ b/tools/xenstore/xenstored_core.c > @@ -523,8 +523,10 @@ static enum xs_perm_type ask_parents(struct connection *conn, const char *name) > } while (!streq(name, "/")); > > /* No permission at root? We''re in trouble. */ > - if (!node) > + if (!node) { > corrupt(conn, "No permissions file at root");Seems almost abort/assert worthy but maybe there is a possible DoS in that (I haven''t looked/thought/checked). So I''ll apply this one.> + return XS_PERM_NONE; > + } > > return perm_for_conn(conn, node->perms, node->num_perms); > }
Matthew Daley
2013-Sep-25 23:11 UTC
Re: [PATCH 17/28] libxl: gettimeofday doesn''t return an errno on failure
On Thu, Sep 26, 2013 at 12:04 AM, Ian Campbell <Ian.Campbell@citrix.com> wrote:> On Wed, 2013-09-18 at 15:37 +1200, Matthew Daley wrote: > > http://pubs.opengroup.org/onlinepubs/9699919799/functions/gettimeofday.html > agrees with this but the Linux manpages gettimeofday(2) disagrees: > gettimeofday() and settimeofday() return 0 for success, or -1 for > failure (in which case errno is set appropriately). > > They may just have confused themselves by lumping get in with set, but > at least one error code (EFAULT) seems like it could apply to get too.I think my lack of description and my use of the "MUST" macro is confusing you here. I agree that they can return values signifying failure, but indeed they don''t return error codes directly, instead they return -1 and set errno.> > Since the spec says it cannot fail I think there''s no harm in reporting > errno if it does fail. > > is the CHK_ERRNO macro correct? > #define CHK_ERRNO( call ) ({ \ > int chk_errno = (call); \ > if (chk_errno < 0) { \ > fprintf(stderr,"xl: fatal error: %s:%d: %s: %s\n", \ > __FILE__,__LINE__, strerror(chk_errno), #call); \ > exit(-ERROR_FAIL); \ > } \ > }) > > It seems to report the reutrn code and not errno, so it will always say > -1 won''t it? > > I think the actual coverity error "chk_errno" is passed to a parameter > that cannot be negative." stems from this not the lack of errno, because > coverity seems to know that strerror cannot take a negative number.Right, and this is what I was trying to avoid by changing to using the MUST macro, but I suppose the lack of errno reportage is unacceptable (and I guess that MUST is meant for internal xl functions that return a exit()able return code, not just -1. Really, I think another macro is needed that gets errno not from the call''s return value, but errno directly? - Matthew
Matthew Daley
2013-Sep-25 23:17 UTC
Re: [PATCH 18/28] xenstored: handle unlikely failure better in ask_parents
On Thu, Sep 26, 2013 at 1:06 AM, Ian Campbell <Ian.Campbell@citrix.com> wrote:> On Wed, 2013-09-18 at 15:37 +1200, Matthew Daley wrote: >> Coverity-ID: 1055277 >> Signed-off-by: Matthew Daley <mattjd@gmail.com> >> --- >> tools/xenstore/xenstored_core.c | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c >> index a2cf2a6..0f8ba64 100644 >> --- a/tools/xenstore/xenstored_core.c >> +++ b/tools/xenstore/xenstored_core.c >> @@ -523,8 +523,10 @@ static enum xs_perm_type ask_parents(struct connection *conn, const char *name) >> } while (!streq(name, "/")); >> >> /* No permission at root? We''re in trouble. */ >> - if (!node) >> + if (!node) { >> corrupt(conn, "No permissions file at root"); > > Seems almost abort/assert worthy but maybe there is a possible DoS in > that (I haven''t looked/thought/checked). So I''ll apply this one.I thought that too, but corrupt() does return after doing actual checks/cleaning on the underlying store (FWIW), so it might be worth letting it flounder a bit longer...> >> + return XS_PERM_NONE; >> + } >> >> return perm_for_conn(conn, node->perms, node->num_perms); >> } > >
Ian Campbell
2013-Sep-26 08:51 UTC
Re: [PATCH 17/28] libxl: gettimeofday doesn''t return an errno on failure
On Thu, 2013-09-26 at 11:11 +1200, Matthew Daley wrote:> On Thu, Sep 26, 2013 at 12:04 AM, Ian Campbell <Ian.Campbell@citrix.com> wrote: > > On Wed, 2013-09-18 at 15:37 +1200, Matthew Daley wrote: > > > > http://pubs.opengroup.org/onlinepubs/9699919799/functions/gettimeofday.html > > agrees with this but the Linux manpages gettimeofday(2) disagrees: > > gettimeofday() and settimeofday() return 0 for success, or -1 for > > failure (in which case errno is set appropriately). > > > > They may just have confused themselves by lumping get in with set, but > > at least one error code (EFAULT) seems like it could apply to get too. > > I think my lack of description and my use of the "MUST" macro is > confusing you here. I agree that they can return values signifying > failure, but indeed they don''t return error codes directly, instead > they return -1 and set errno. > > > > > Since the spec says it cannot fail I think there''s no harm in reporting > > errno if it does fail. > > > > is the CHK_ERRNO macro correct? > > #define CHK_ERRNO( call ) ({ \ > > int chk_errno = (call); \ > > if (chk_errno < 0) { \ > > fprintf(stderr,"xl: fatal error: %s:%d: %s: %s\n", \ > > __FILE__,__LINE__, strerror(chk_errno), #call); \ > > exit(-ERROR_FAIL); \ > > } \ > > }) > > > > It seems to report the reutrn code and not errno, so it will always say > > -1 won''t it? > > > > I think the actual coverity error "chk_errno" is passed to a parameter > > that cannot be negative." stems from this not the lack of errno, because > > coverity seems to know that strerror cannot take a negative number. > > Right, and this is what I was trying to avoid by changing to using the > MUST macro, but I suppose the lack of errno reportage is unacceptable > (and I guess that MUST is meant for internal xl functions that return > a exit()able return code, not just -1. Really, I think another macro > is needed that gets errno not from the call''s return value, but errno > directly?I think the problem is that CHK_ERRNO is actually used to check both libc functions (which return -1 and set errno) and libxl function (which return -LIBXL_ERROR), and does neither of them properly: xl_cmdimpl.c: CHK_ERRNO( libxl_read_exactly(ctx, restore_f xl_cmdimpl.c: CHK_ERRNO( libxl_read_exactly(ctx, resto xl_cmdimpl.c: CHK_ERRNO(( logfile = open(fullname, O_WRONL xl_cmdimpl.c: CHK_ERRNO(( nullfd = open("/dev/null", O_RDO xl_cmdimpl.c: CHK_ERRNO(daemon(0, 1) < 0); xl_cmdimpl.c: CHK_ERRNO(asprintf(&config_source, "<domid % xl_cmdimpl.c: CHK_ERRNO( libxl_write_exactly(ctx, fd, xl_cmdimpl.c: CHK_ERRNO( libxl_write_exactly(ctx, fd, xl_cmdimpl.c: CHK_ERRNO( gettimeofday(&waituntil, 0) ); xl_cmdimpl.c: CHK_ERRNO( gettimeofday(&now, 0) ); xl_cmdimpl.c: CHK_ERRNO( libxl_write_exactly(ctx, send_fd, When used with libc functions it takes the return code (always -1 on error) and calls strerror on it to get whatever errno -1 happens to be, which is bogus. When used with libxl functions it takes the libxl error code (which is not an errno) and passes it to strerror, which is also bogus. I think we need to change CHK_ERRNO to strerror(errno) and introduce CHK_LIBXL which does the right thing for libxl error codes and then use CHK_ERRNO for libc functions and CHK_LIBXL for libxl ones... Or maybe MUST is already the CHK_LIBXL we want, but in that case using MUST on libc functions is wrong... Ian.
Boris Ostrovsky
2013-Sep-26 19:29 UTC
Re: [PATCH 04/28] libxl: fix libxl_string_list_length and its only caller
On 09/17/2013 11:37 PM, Matthew Daley wrote:> The wrong amount of indirections were being taken in > libxl_string_list_length, and its only caller was miscounting the amount > of initial non-list arguments, seemingly since the initial commit > (599c784). > > This has been seen and reported in the wild (##xen): > < Trixboxer> Hi, any idea why would I get > < Trixboxer> xl: libxl_bootloader.c:42: bootloader_arg: Assertion `bl->nargs < bl->argsspace'' failed. > < Trixboxer> 4.2.2-23.el6 > > Coverity-ID: 1054954 > Signed-off-by: Matthew Daley<mattjd@gmail.com> > --- > tools/libxl/libxl.c | 2 +- > tools/libxl/libxl_bootloader.c | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c > index 0879f23..ca24ca3 100644 > --- a/tools/libxl/libxl.c > +++ b/tools/libxl/libxl.c > @@ -202,7 +202,7 @@ int libxl_string_list_length(const libxl_string_list *psl) > { > if (!psl) return 0;This should be if (!psl || !(*psl)) return 0; We segfault otherwise at the changed line below if no arguments are passed to the bootloader (as is the case with pvgrub).> int i = 0; > - while (*psl++) i++; > + while ((*psl)[i]) i++; > return i; > }(I am surprised Coverity didn''t flag this) -boris> > diff --git a/tools/libxl/libxl_bootloader.c b/tools/libxl/libxl_bootloader.c > index ed12b2c..3287bf7 100644 > --- a/tools/libxl/libxl_bootloader.c > +++ b/tools/libxl/libxl_bootloader.c > @@ -48,7 +48,7 @@ static void make_bootloader_args(libxl__gc *gc, libxl__bootloader_state *bl, > { > const libxl_domain_build_info *info = bl->info; > > - bl->argsspace = 7 + libxl_string_list_length(&info->u.pv.bootloader_args); > + bl->argsspace = 9 + libxl_string_list_length(&info->u.pv.bootloader_args); > > GCNEW_ARRAY(bl->args, bl->argsspace); >
Andrew Cooper
2013-Sep-26 19:37 UTC
Re: [PATCH 04/28] libxl: fix libxl_string_list_length and its only caller
On 26/09/13 20:29, Boris Ostrovsky wrote:> On 09/17/2013 11:37 PM, Matthew Daley wrote: >> The wrong amount of indirections were being taken in >> libxl_string_list_length, and its only caller was miscounting the amount >> of initial non-list arguments, seemingly since the initial commit >> (599c784). >> >> This has been seen and reported in the wild (##xen): >> < Trixboxer> Hi, any idea why would I get >> < Trixboxer> xl: libxl_bootloader.c:42: bootloader_arg: Assertion >> `bl->nargs < bl->argsspace'' failed. >> < Trixboxer> 4.2.2-23.el6 >> >> Coverity-ID: 1054954 >> Signed-off-by: Matthew Daley<mattjd@gmail.com> >> --- >> tools/libxl/libxl.c | 2 +- >> tools/libxl/libxl_bootloader.c | 2 +- >> 2 files changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c >> index 0879f23..ca24ca3 100644 >> --- a/tools/libxl/libxl.c >> +++ b/tools/libxl/libxl.c >> @@ -202,7 +202,7 @@ int libxl_string_list_length(const >> libxl_string_list *psl) >> { >> if (!psl) return 0; > > This should be > if (!psl || !(*psl)) return 0; > > We segfault otherwise at the changed line below if no arguments are > passed to the bootloader (as is the case with pvgrub). > >> int i = 0; >> - while (*psl++) i++; >> + while ((*psl)[i]) i++; >> return i; >> } > > (I am surprised Coverity didn''t flag this) > > -borisFor this bug, Coveritys specific objection was that the caller passes a singleton pointer into this function, and this function uses it as an array. This is a side effect of using the wrong indirection. However, your spot of the crash is good, and needs fixing as well. ~Andrew
Matthew Daley
2013-Sep-27 00:46 UTC
Re: [PATCH 04/28] libxl: fix libxl_string_list_length and its only caller
On Fri, Sep 27, 2013 at 7:37 AM, Andrew Cooper <andrew.cooper3@citrix.com> wrote:> On 26/09/13 20:29, Boris Ostrovsky wrote: >> On 09/17/2013 11:37 PM, Matthew Daley wrote: >>> The wrong amount of indirections were being taken in >>> libxl_string_list_length, and its only caller was miscounting the amount >>> of initial non-list arguments, seemingly since the initial commit >>> (599c784). >>> >>> This has been seen and reported in the wild (##xen): >>> < Trixboxer> Hi, any idea why would I get >>> < Trixboxer> xl: libxl_bootloader.c:42: bootloader_arg: Assertion >>> `bl->nargs < bl->argsspace'' failed. >>> < Trixboxer> 4.2.2-23.el6 >>> >>> Coverity-ID: 1054954 >>> Signed-off-by: Matthew Daley<mattjd@gmail.com> >>> --- >>> tools/libxl/libxl.c | 2 +- >>> tools/libxl/libxl_bootloader.c | 2 +- >>> 2 files changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c >>> index 0879f23..ca24ca3 100644 >>> --- a/tools/libxl/libxl.c >>> +++ b/tools/libxl/libxl.c >>> @@ -202,7 +202,7 @@ int libxl_string_list_length(const >>> libxl_string_list *psl) >>> { >>> if (!psl) return 0; >> >> This should be >> if (!psl || !(*psl)) return 0; >> >> We segfault otherwise at the changed line below if no arguments are >> passed to the bootloader (as is the case with pvgrub). >> >>> int i = 0; >>> - while (*psl++) i++; >>> + while ((*psl)[i]) i++; >>> return i; >>> } >> >> (I am surprised Coverity didn''t flag this) >> >> -boris > > For this bug, Coveritys specific objection was that the caller passes a > singleton pointer into this function, and this function uses it as an > array. This is a side effect of using the wrong indirection.Also, we don''t have automated scanning set up yet, and this patch is still in staging anyway.> > However, your spot of the crash is good, and needs fixing as well.Indeed, I''ll send out a patch. - Matthew> > ~Andrew
Matthew Daley
2013-Sep-29 01:35 UTC
[PATCH] libxc: only munmap when something has actually been mapped in change_pte
Coverity-ID: 1055269
Signed-off-by: Matthew Daley <mattjd@gmail.com>
---
How''s this? It''s hard to create a very meaningful commit title
here, IMO...
tools/libxc/xc_offline_page.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/libxc/xc_offline_page.c b/tools/libxc/xc_offline_page.c
index fbb53f5..8195efb 100644
--- a/tools/libxc/xc_offline_page.c
+++ b/tools/libxc/xc_offline_page.c
@@ -317,10 +317,10 @@ static int change_pte(xc_interface *xch, int domid,
goto failed;
}
}
- }
- munmap(content, PAGE_SIZE);
- content = NULL;
+ munmap(content, PAGE_SIZE);
+ content = NULL;
+ }
}
if ( xc_flush_mmu_updates(xch, mmu) )
--
1.7.10.4
Matthew Daley
2013-Sep-29 03:05 UTC
[PATCH] remove unnecessary null pointer checks before frees
So this is just an RFC for now, but I went ahead and corrected all instances
of such unnecessary null pointer checks before frees across the entire tree
that were in 1st-party code (I think).
Not sure if this would be considered helpful, or just annoying noise :)
Build-tested.
-- 8< --
Patch generated by the following semantic patch
(http://coccinelle.lip6.fr/):
@@
expression *P;
@@
- if(P) free(P);
+ free(P);
...and then by filtering through the following command:
filterdiff -p1 -x ''stubdom/*'' -x
''tools/firmware/*'' -x ''tools/qemu-*''
Signed-off-by: Matthew Daley <mattjd@gmail.com>
---
extras/mini-os/blkfront.c | 14 ++++++-------
extras/mini-os/console/xenbus.c | 8 +++----
extras/mini-os/fbfront.c | 38 +++++++++++++++++-----------------
extras/mini-os/kernel.c | 4 ++--
extras/mini-os/lib/sys.c | 3 +--
extras/mini-os/netfront.c | 18 ++++++++--------
extras/mini-os/pcifront.c | 14 ++++++-------
tools/blktap/drivers/blktapctrl.c | 6 ++----
tools/blktap/drivers/tapaio.c | 18 ++++++----------
tools/blktap/drivers/tapdisk.c | 9 +++-----
tools/blktap/lib/xenbus.c | 21 +++++++------------
tools/blktap/lib/xs_api.c | 3 +--
tools/blktap2/control/tap-ctl-list.c | 6 ++----
tools/blktap2/drivers/block-log.c | 3 +--
tools/flask/utils/loadpolicy.c | 3 +--
tools/libxc/xc_compression.c | 15 +++++---------
tools/libxc/xc_core_x86.c | 6 ++----
tools/libxc/xc_domain_save.c | 6 ++----
tools/libxc/xc_gnttab.c | 3 +--
tools/libxc/xc_offline_page.c | 9 +++-----
tools/libxl/libxl_event.c | 3 +--
tools/libxl/libxl_qmp.c | 6 ++----
tools/libxl/libxl_utils.c | 2 +-
tools/memshr/bidir-hash.c | 4 ++--
tools/misc/gtraceview.c | 8 ++-----
tools/tests/xen-access/xen-access.c | 6 ++----
tools/xenbackendd/xenbackendd.c | 6 ++----
27 files changed, 97 insertions(+), 145 deletions(-)
diff --git a/extras/mini-os/blkfront.c b/extras/mini-os/blkfront.c
index aead6bd..62a32c5 100644
--- a/extras/mini-os/blkfront.c
+++ b/extras/mini-os/blkfront.c
@@ -160,7 +160,7 @@ again:
err = xenbus_transaction_end(xbt, 0, &retry);
- if (err) free(err);
+ free(err);
if (retry) {
goto again;
printk("completing transaction\n");
@@ -271,7 +271,7 @@ void shutdown_blkfront(struct blkfront_dev *dev)
state = xenbus_read_integer(path);
while (err == NULL && state < XenbusStateClosing)
err = xenbus_wait_for_state_change(path, &state,
&dev->events);
- if (err) free(err);
+ free(err);
if ((err = xenbus_switch_state(XBT_NIL, nodename, XenbusStateClosed)) !=
NULL) {
printk("shutdown_blkfront: error changing state to %d: %s\n",
@@ -281,7 +281,7 @@ void shutdown_blkfront(struct blkfront_dev *dev)
state = xenbus_read_integer(path);
while (state < XenbusStateClosed) {
err = xenbus_wait_for_state_change(path, &state,
&dev->events);
- if (err) free(err);
+ free(err);
}
if ((err = xenbus_switch_state(XBT_NIL, nodename, XenbusStateInitialising))
!= NULL) {
@@ -294,16 +294,16 @@ void shutdown_blkfront(struct blkfront_dev *dev)
err = xenbus_wait_for_state_change(path, &state,
&dev->events);
close:
- if (err) free(err);
+ free(err);
err2 = xenbus_unwatch_path_token(XBT_NIL, path, path);
- if (err2) free(err2);
+ free(err2);
snprintf(nodename, sizeof(nodename), "%s/ring-ref",
dev->nodename);
err2 = xenbus_rm(XBT_NIL, nodename);
- if (err2) free(err2);
+ free(err2);
snprintf(nodename, sizeof(nodename), "%s/event-channel",
dev->nodename);
err2 = xenbus_rm(XBT_NIL, nodename);
- if (err2) free(err2);
+ free(err2);
if (!err)
free_blkfront(dev);
diff --git a/extras/mini-os/console/xenbus.c b/extras/mini-os/console/xenbus.c
index b317114..1c9a590 100644
--- a/extras/mini-os/console/xenbus.c
+++ b/extras/mini-os/console/xenbus.c
@@ -32,7 +32,7 @@ void free_consfront(struct consfront_dev *dev)
state = xenbus_read_integer(path);
while (err == NULL && state < XenbusStateClosing)
err = xenbus_wait_for_state_change(path, &state,
&dev->events);
- if (err) free(err);
+ free(err);
if ((err = xenbus_switch_state(XBT_NIL, nodename, XenbusStateClosed)) !=
NULL) {
printk("free_consfront: error changing state to %d: %s\n",
@@ -41,9 +41,9 @@ void free_consfront(struct consfront_dev *dev)
}
close:
- if (err) free(err);
+ free(err);
err = xenbus_unwatch_path_token(XBT_NIL, path, path);
- if (err) free(err);
+ free(err);
mask_evtchn(dev->evtchn);
unbind_evtchn(dev->evtchn);
@@ -134,7 +134,7 @@ again:
err = xenbus_transaction_end(xbt, 0, &retry);
- if (err) free(err);
+ free(err);
if (retry) {
goto again;
printk("completing transaction\n");
diff --git a/extras/mini-os/fbfront.c b/extras/mini-os/fbfront.c
index a276193..1e01513 100644
--- a/extras/mini-os/fbfront.c
+++ b/extras/mini-os/fbfront.c
@@ -131,7 +131,7 @@ again:
}
err = xenbus_transaction_end(xbt, 0, &retry);
- if (err) free(err);
+ free(err);
if (retry) {
goto again;
printk("completing transaction\n");
@@ -171,7 +171,7 @@ done:
err = xenbus_wait_for_state_change(path, &state,
&dev->events);
if (state != XenbusStateConnected) {
printk("backend not available, state=%d\n", state);
- if (err) free(err);
+ free(err);
err = xenbus_unwatch_path_token(XBT_NIL, path, path);
goto error;
}
@@ -256,7 +256,7 @@ void shutdown_kbdfront(struct kbdfront_dev *dev)
state = xenbus_read_integer(path);
while (err == NULL && state < XenbusStateClosing)
err = xenbus_wait_for_state_change(path, &state,
&dev->events);
- if (err) free(err);
+ free(err);
if ((err = xenbus_switch_state(XBT_NIL, nodename, XenbusStateClosed)) !=
NULL) {
printk("shutdown_kbdfront: error changing state to %d: %s\n",
@@ -266,7 +266,7 @@ void shutdown_kbdfront(struct kbdfront_dev *dev)
state = xenbus_read_integer(path);
while (state < XenbusStateClosed) {
err = xenbus_wait_for_state_change(path, &state,
&dev->events);
- if (err) free(err);
+ free(err);
}
if ((err = xenbus_switch_state(XBT_NIL, nodename, XenbusStateInitialising))
!= NULL) {
@@ -279,19 +279,19 @@ void shutdown_kbdfront(struct kbdfront_dev *dev)
err = xenbus_wait_for_state_change(path, &state, &dev->events);
close_kbdfront:
- if (err) free(err);
+ free(err);
err2 = xenbus_unwatch_path_token(XBT_NIL, path, path);
- if (err2) free(err2);
+ free(err2);
snprintf(nodename, sizeof(nodename), "%s/page-ref",
dev->nodename);
err2 = xenbus_rm(XBT_NIL, nodename);
- if (err2) free(err2);
+ free(err2);
snprintf(nodename, sizeof(nodename), "%s/event-channel",
dev->nodename);
err2 = xenbus_rm(XBT_NIL, nodename);
- if (err2) free(err2);
+ free(err2);
snprintf(nodename, sizeof(nodename), "%s/request-abs-pointer",
dev->nodename);
err2 = xenbus_rm(XBT_NIL, nodename);
- if (err2) free(err2);
+ free(err2);
if (!err)
free_kbdfront(dev);
@@ -498,7 +498,7 @@ again:
}
err = xenbus_transaction_end(xbt, 0, &retry);
- if (err) free(err);
+ free(err);
if (retry) {
goto again;
printk("completing transaction\n");
@@ -538,7 +538,7 @@ done:
err = xenbus_wait_for_state_change(path, &state,
&dev->events);
if (state != XenbusStateConnected) {
printk("backend not available, state=%d\n", state);
- if (err) free(err);
+ free(err);
err = xenbus_unwatch_path_token(XBT_NIL, path, path);
goto error;
}
@@ -654,7 +654,7 @@ void shutdown_fbfront(struct fbfront_dev *dev)
state = xenbus_read_integer(path);
while (err == NULL && state < XenbusStateClosing)
err = xenbus_wait_for_state_change(path, &state,
&dev->events);
- if (err) free(err);
+ free(err);
if ((err = xenbus_switch_state(XBT_NIL, nodename, XenbusStateClosed)) !=
NULL) {
printk("shutdown_fbfront: error changing state to %d: %s\n",
@@ -664,7 +664,7 @@ void shutdown_fbfront(struct fbfront_dev *dev)
state = xenbus_read_integer(path);
if (state < XenbusStateClosed) {
err = xenbus_wait_for_state_change(path, &state,
&dev->events);
- if (err) free(err);
+ free(err);
}
if ((err = xenbus_switch_state(XBT_NIL, nodename, XenbusStateInitialising))
!= NULL) {
@@ -677,22 +677,22 @@ void shutdown_fbfront(struct fbfront_dev *dev)
err = xenbus_wait_for_state_change(path, &state,
&dev->events);
close_fbfront:
- if (err) free(err);
+ free(err);
err2 = xenbus_unwatch_path_token(XBT_NIL, path, path);
- if (err2) free(err2);
+ free(err2);
snprintf(nodename, sizeof(nodename), "%s/page-ref",
dev->nodename);
err2 = xenbus_rm(XBT_NIL, nodename);
- if (err2) free(err2);
+ free(err2);
snprintf(nodename, sizeof(nodename), "%s/event-channel",
dev->nodename);
err2 = xenbus_rm(XBT_NIL, nodename);
- if (err2) free(err2);
+ free(err2);
snprintf(nodename, sizeof(nodename), "%s/protocol",
dev->nodename);
err2 = xenbus_rm(XBT_NIL, nodename);
- if (err2) free(err2);
+ free(err2);
snprintf(nodename, sizeof(nodename), "%s/feature-update",
dev->nodename);
err2 = xenbus_rm(XBT_NIL, nodename);
- if (err2) free(err2);
+ free(err2);
if (!err)
free_fbfront(dev);
diff --git a/extras/mini-os/kernel.c b/extras/mini-os/kernel.c
index 24fa25c..ea409f4 100644
--- a/extras/mini-os/kernel.c
+++ b/extras/mini-os/kernel.c
@@ -85,9 +85,9 @@ static void shutdown_thread(void *p)
xenbus_wait_for_watch(&events);
}
err = xenbus_unwatch_path_token(XBT_NIL, path, token);
- if (err) free(err);
+ free(err);
err = xenbus_write(XBT_NIL, path, "");
- if (err) free(err);
+ free(err);
printk("Shutting down (%s)\n", shutdown);
if (!strcmp(shutdown, "poweroff"))
diff --git a/extras/mini-os/lib/sys.c b/extras/mini-os/lib/sys.c
index cfbdc90..13e7e59 100644
--- a/extras/mini-os/lib/sys.c
+++ b/extras/mini-os/lib/sys.c
@@ -1156,8 +1156,7 @@ LWIP_STUB(int, getsockname, (int s, struct sockaddr *name,
socklen_t *namelen),
static char *syslog_ident;
void openlog(const char *ident, int option, int facility)
{
- if (syslog_ident)
- free(syslog_ident);
+ free(syslog_ident);
syslog_ident = strdup(ident);
}
diff --git a/extras/mini-os/netfront.c b/extras/mini-os/netfront.c
index 3a5be64..44c3995 100644
--- a/extras/mini-os/netfront.c
+++ b/extras/mini-os/netfront.c
@@ -412,7 +412,7 @@ again:
}
err = xenbus_transaction_end(xbt, 0, &retry);
- if (err) free(err);
+ free(err);
if (retry) {
goto again;
printk("completing transaction\n");
@@ -525,7 +525,7 @@ void shutdown_netfront(struct netfront_dev *dev)
state = xenbus_read_integer(path);
while (err == NULL && state < XenbusStateClosing)
err = xenbus_wait_for_state_change(path, &state,
&dev->events);
- if (err) free(err);
+ free(err);
if ((err = xenbus_switch_state(XBT_NIL, nodename, XenbusStateClosed)) !=
NULL) {
printk("shutdown_netfront: error changing state to %d: %s\n",
@@ -535,7 +535,7 @@ void shutdown_netfront(struct netfront_dev *dev)
state = xenbus_read_integer(path);
while (state < XenbusStateClosed) {
err = xenbus_wait_for_state_change(path, &state,
&dev->events);
- if (err) free(err);
+ free(err);
}
if ((err = xenbus_switch_state(XBT_NIL, nodename, XenbusStateInitialising))
!= NULL) {
@@ -548,22 +548,22 @@ void shutdown_netfront(struct netfront_dev *dev)
err = xenbus_wait_for_state_change(path, &state,
&dev->events);
close:
- if (err) free(err);
+ free(err);
err2 = xenbus_unwatch_path_token(XBT_NIL, path, path);
- if (err2) free(err2);
+ free(err2);
snprintf(nodename, sizeof(nodename), "%s/tx-ring-ref",
dev->nodename);
err2 = xenbus_rm(XBT_NIL, nodename);
- if (err2) free(err2);
+ free(err2);
snprintf(nodename, sizeof(nodename), "%s/rx-ring-ref",
dev->nodename);
err2 = xenbus_rm(XBT_NIL, nodename);
- if (err2) free(err2);
+ free(err2);
snprintf(nodename, sizeof(nodename), "%s/event-channel",
dev->nodename);
err2 = xenbus_rm(XBT_NIL, nodename);
- if (err2) free(err2);
+ free(err2);
snprintf(nodename, sizeof(nodename), "%s/request-rx-copy",
dev->nodename);
err2 = xenbus_rm(XBT_NIL, nodename);
- if (err2) free(err2);
+ free(err2);
if (!err)
free_netfront(dev);
diff --git a/extras/mini-os/pcifront.c b/extras/mini-os/pcifront.c
index 7838021..16a4b49 100644
--- a/extras/mini-os/pcifront.c
+++ b/extras/mini-os/pcifront.c
@@ -212,7 +212,7 @@ again:
}
err = xenbus_transaction_end(xbt, 0, &retry);
- if (err) free(err);
+ free(err);
if (retry) {
goto again;
printk("completing transaction\n");
@@ -251,7 +251,7 @@ done:
err = xenbus_wait_for_state_change(path, &state,
&dev->events);
if (state != XenbusStateConnected) {
printk("backend not avalable, state=%d\n", state);
- if (err) free(err);
+ free(err);
err = xenbus_unwatch_path_token(XBT_NIL, path, path);
goto error;
}
@@ -342,7 +342,7 @@ void shutdown_pcifront(struct pcifront_dev *dev)
state = xenbus_read_integer(path);
while (err == NULL && state < XenbusStateClosing)
err = xenbus_wait_for_state_change(path, &state,
&dev->events);
- if (err) free(err);
+ free(err);
if ((err = xenbus_switch_state(XBT_NIL, nodename, XenbusStateClosed)) !=
NULL) {
printk("shutdown_pcifront: error changing state to %d: %s\n",
@@ -365,16 +365,16 @@ void shutdown_pcifront(struct pcifront_dev *dev)
err = xenbus_wait_for_state_change(path, &state,
&dev->events);
close_pcifront:
- if (err) free(err);
+ free(err);
err2 = xenbus_unwatch_path_token(XBT_NIL, path, path);
- if (err2) free(err2);
+ free(err2);
snprintf(nodename, sizeof(nodename), "%s/info-ref",
dev->nodename);
err2 = xenbus_rm(XBT_NIL, nodename);
- if (err2) free(err2);
+ free(err2);
snprintf(nodename, sizeof(nodename), "%s/event-channel",
dev->nodename);
err2 = xenbus_rm(XBT_NIL, nodename);
- if (err2) free(err2);
+ free(err2);
if (!err)
free_pcifront(dev);
diff --git a/tools/blktap/drivers/blktapctrl.c
b/tools/blktap/drivers/blktapctrl.c
index 0a8b880..0022264 100644
--- a/tools/blktap/drivers/blktapctrl.c
+++ b/tools/blktap/drivers/blktapctrl.c
@@ -642,11 +642,9 @@ static int connect_tapdisk(blkif_t *blkif, int minor)
ret = 0;
fail:
- if (rdctldev)
- free(rdctldev);
+ free(rdctldev);
- if (wrctldev)
- free(wrctldev);
+ free(wrctldev);
return ret;
}
diff --git a/tools/blktap/drivers/tapaio.c b/tools/blktap/drivers/tapaio.c
index 140c44a..ae26577 100644
--- a/tools/blktap/drivers/tapaio.c
+++ b/tools/blktap/drivers/tapaio.c
@@ -244,18 +244,12 @@ fail:
void tap_aio_free(tap_aio_context_t *ctx)
{
- if (ctx->sector_lock)
- free(ctx->sector_lock);
- if (ctx->iocb_list)
- free(ctx->iocb_list);
- if (ctx->pending_aio)
- free(ctx->pending_aio);
- if (ctx->aio_events)
- free(ctx->aio_events);
- if (ctx->iocb_free)
- free(ctx->iocb_free);
- if (ctx->iocb_queue)
- free(ctx->iocb_queue);
+ free(ctx->sector_lock);
+ free(ctx->iocb_list);
+ free(ctx->pending_aio);
+ free(ctx->aio_events);
+ free(ctx->iocb_free);
+ free(ctx->iocb_queue);
}
/*TODO: Fix sector span!*/
diff --git a/tools/blktap/drivers/tapdisk.c b/tools/blktap/drivers/tapdisk.c
index 19cd777..a6e8a7c 100644
--- a/tools/blktap/drivers/tapdisk.c
+++ b/tools/blktap/drivers/tapdisk.c
@@ -82,10 +82,8 @@ static void daemonize(void)
static void free_driver(struct disk_driver *d)
{
- if (d->name)
- free(d->name);
- if (d->private)
- free(d->private);
+ free(d->name);
+ free(d->private);
free(d);
}
@@ -354,8 +352,7 @@ static int open_disk(struct td_state *s,
fail:
DPRINTF("failed opening disk\n");
- if (id.name)
- free(id.name);
+ free(id.name);
d = s->disks;
while (d) {
struct disk_driver *tmp = d->next;
diff --git a/tools/blktap/lib/xenbus.c b/tools/blktap/lib/xenbus.c
index 948eb02..9edbf5a 100644
--- a/tools/blktap/lib/xenbus.c
+++ b/tools/blktap/lib/xenbus.c
@@ -160,10 +160,8 @@ static int backend_remove(struct xs_handle *h, struct
backend_info *be)
DPRINTF("Freeing blkif dev [%d]\n",be->blkif->devnum);
free_blkif(be->blkif);
}
- if (be->frontpath)
- free(be->frontpath);
- if (be->backpath)
- free(be->backpath);
+ free(be->frontpath);
+ free(be->backpath);
free(be);
return 0;
}
@@ -435,8 +433,7 @@ fail:
backend_remove(h, be);
}
close:
- if (path)
- free(path);
+ free(path);
return;
}
@@ -501,8 +498,7 @@ static void ueblktap_probe(struct xs_handle *h, struct
xenbus_watch *w,
if ( (be != NULL) && (be->blkif != NULL) )
backend_remove(h, be);
else goto free_be;
- if (bepath)
- free(bepath);
+ free(bepath);
return;
}
@@ -522,12 +518,9 @@ static void ueblktap_probe(struct xs_handle *h, struct
xenbus_watch *w,
return;
free_be:
- if (frontend)
- free(frontend);
- if (bepath)
- free(bepath);
- if (be)
- free(be);
+ free(frontend);
+ free(bepath);
+ free(be);
}
/**
diff --git a/tools/blktap/lib/xs_api.c b/tools/blktap/lib/xs_api.c
index 4648432..1624cb9 100644
--- a/tools/blktap/lib/xs_api.c
+++ b/tools/blktap/lib/xs_api.c
@@ -204,8 +204,7 @@ char *get_dom_domid(struct xs_handle *h)
}
done:
xs_transaction_end(h, xth, 0);
- if (e)
- free(e);
+ free(e);
return domid;
}
diff --git a/tools/blktap2/control/tap-ctl-list.c
b/tools/blktap2/control/tap-ctl-list.c
index c91f6f4..170735e 100644
--- a/tools/blktap2/control/tap-ctl-list.c
+++ b/tools/blktap2/control/tap-ctl-list.c
@@ -219,8 +219,7 @@ out:
return err ? : n_minors;
fail:
- if (minorv)
- free(minorv);
+ free(minorv);
goto out;
}
@@ -302,8 +301,7 @@ out:
return err ? : n_taps;
fail:
- if (tapv)
- free(tapv);
+ free(tapv);
goto out;
}
diff --git a/tools/blktap2/drivers/block-log.c
b/tools/blktap2/drivers/block-log.c
index 5330cdc..21c1946 100644
--- a/tools/blktap2/drivers/block-log.c
+++ b/tools/blktap2/drivers/block-log.c
@@ -110,8 +110,7 @@ static int writelog_create(struct tdlog_state *s)
static int writelog_free(struct tdlog_state *s)
{
- if (s->writelog)
- free(s->writelog);
+ free(s->writelog);
return 0;
}
diff --git a/tools/flask/utils/loadpolicy.c b/tools/flask/utils/loadpolicy.c
index f347b97..2edcf97 100644
--- a/tools/flask/utils/loadpolicy.c
+++ b/tools/flask/utils/loadpolicy.c
@@ -108,8 +108,7 @@ int main (int argCnt, const char *args[])
}
done:
- if ( polMemCp )
- free(polMemCp);
+ free(polMemCp);
if ( polMem )
{
ret = munmap(polMem, info.st_size);
diff --git a/tools/libxc/xc_compression.c b/tools/libxc/xc_compression.c
index 89f1be7..8f0b89d 100644
--- a/tools/libxc/xc_compression.c
+++ b/tools/libxc/xc_compression.c
@@ -457,16 +457,11 @@ void xc_compression_free_context(xc_interface *xch,
comp_ctx *ctx)
{
if (!ctx) return;
- if (ctx->inputbuf)
- free(ctx->inputbuf);
- if (ctx->sendbuf_pfns)
- free(ctx->sendbuf_pfns);
- if (ctx->cache_base)
- free(ctx->cache_base);
- if (ctx->pfn2cache)
- free(ctx->pfn2cache);
- if (ctx->cache)
- free(ctx->cache);
+ free(ctx->inputbuf);
+ free(ctx->sendbuf_pfns);
+ free(ctx->cache_base);
+ free(ctx->pfn2cache);
+ free(ctx->cache);
free(ctx);
}
diff --git a/tools/libxc/xc_core_x86.c b/tools/libxc/xc_core_x86.c
index a9681f7..e328dcf 100644
--- a/tools/libxc/xc_core_x86.c
+++ b/tools/libxc/xc_core_x86.c
@@ -180,11 +180,9 @@ out:
if ( live_p2m_frame_list )
munmap(live_p2m_frame_list, P2M_FLL_ENTRIES * PAGE_SIZE);
- if ( p2m_frame_list_list )
- free(p2m_frame_list_list);
+ free(p2m_frame_list_list);
- if ( p2m_frame_list )
- free(p2m_frame_list);
+ free(p2m_frame_list);
errno = err;
return ret;
diff --git a/tools/libxc/xc_domain_save.c b/tools/libxc/xc_domain_save.c
index fbc15e9..673d1e9 100644
--- a/tools/libxc/xc_domain_save.c
+++ b/tools/libxc/xc_domain_save.c
@@ -773,11 +773,9 @@ static xen_pfn_t *map_and_save_p2m_table(xc_interface *xch,
if ( live_p2m_frame_list )
munmap(live_p2m_frame_list, P2M_FLL_ENTRIES * PAGE_SIZE);
- if ( p2m_frame_list_list )
- free(p2m_frame_list_list);
+ free(p2m_frame_list_list);
- if ( p2m_frame_list )
- free(p2m_frame_list);
+ free(p2m_frame_list);
return success ? p2m : NULL;
}
diff --git a/tools/libxc/xc_gnttab.c b/tools/libxc/xc_gnttab.c
index a8cc381..79dab40 100644
--- a/tools/libxc/xc_gnttab.c
+++ b/tools/libxc/xc_gnttab.c
@@ -124,8 +124,7 @@ static void *_gnttab_map_table(xc_interface *xch, int domid,
int *gnt_num)
err:
if ( frame_list )
xc_hypercall_buffer_free(xch, frame_list);
- if ( pfn_list )
- free(pfn_list);
+ free(pfn_list);
return gnt;
}
diff --git a/tools/libxc/xc_offline_page.c b/tools/libxc/xc_offline_page.c
index 8195efb..0b4cdf9 100644
--- a/tools/libxc/xc_offline_page.c
+++ b/tools/libxc/xc_offline_page.c
@@ -626,14 +626,11 @@ failed:
}
}
- if (mmu)
- free(mmu);
+ free(mmu);
- if (old_ptes.entries)
- free(old_ptes.entries);
+ free(old_ptes.entries);
- if (backup)
- free(backup);
+ free(backup);
if (gnttab_v1)
munmap(gnttab_v1, gnt_num / (PAGE_SIZE/sizeof(grant_entry_v1_t)));
diff --git a/tools/libxl/libxl_event.c b/tools/libxl/libxl_event.c
index fd2f280..73b7e63 100644
--- a/tools/libxl/libxl_event.c
+++ b/tools/libxl/libxl_event.c
@@ -578,8 +578,7 @@ int libxl__ev_xswatch_register(libxl__gc *gc,
libxl__ev_xswatch *w,
out_rc:
if (use)
LIBXL_SLIST_INSERT_HEAD(&CTX->watch_freeslots, use, empty);
- if (path_copy)
- free(path_copy);
+ free(path_copy);
CTX_UNLOCK;
return rc;
}
diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
index f681f3a..7e825ee 100644
--- a/tools/libxl/libxl_qmp.c
+++ b/tools/libxl/libxl_qmp.c
@@ -406,12 +406,10 @@ static void qmp_close(libxl__qmp_handler *qmp)
close(qmp->qmp_fd);
LIBXL_STAILQ_FOREACH(pp, &qmp->callback_list, next) {
- if (tmp)
- free(tmp);
+ free(tmp);
tmp = pp;
}
- if (tmp)
- free(tmp);
+ free(tmp);
}
static int qmp_next(libxl__gc *gc, libxl__qmp_handler *qmp)
diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
index 8e567a8..664a730 100644
--- a/tools/libxl/libxl_utils.c
+++ b/tools/libxl/libxl_utils.c
@@ -355,7 +355,7 @@ int libxl_read_file_contents(libxl_ctx *ctx, const char
*filename,
e = errno;
assert(e != ENOENT);
if (f) fclose(f);
- if (data) free(data);
+ free(data);
return e;
}
diff --git a/tools/memshr/bidir-hash.c b/tools/memshr/bidir-hash.c
index bed8179..3d34637 100644
--- a/tools/memshr/bidir-hash.c
+++ b/tools/memshr/bidir-hash.c
@@ -208,8 +208,8 @@ static void free_buckets(struct __hash *h,
struct bucket *buckets,
struct bucket_lock *bucket_locks)
{
- if(buckets) free(buckets);
- if(bucket_locks) free(bucket_locks);
+ free(buckets);
+ free(bucket_locks);
}
static int max_entries(struct __hash *h)
diff --git a/tools/misc/gtraceview.c b/tools/misc/gtraceview.c
index d8b4589..72a3abc 100644
--- a/tools/misc/gtraceview.c
+++ b/tools/misc/gtraceview.c
@@ -959,8 +959,7 @@ int time_mode_rebuild(uint64_t start_time, uint64_t
time_scale)
state = malloc(sizeof(struct state) * number);
if (!state)
return 1;
- if (this->state)
- free(this->state);
+ free(this->state);
this->state = state;
this->width = 9;
this->row = 0;
@@ -978,10 +977,7 @@ int time_mode_rebuild(uint64_t start_time, uint64_t
time_scale)
int num[MAX_CPU_NR];
int last_idx[MAX_CPU_NR];
-#if 0
- printf("XXXXX %d tsc: %"PRIu64" data[i].tsc:
%"PRIu64"\n",
- i, tsc, data[i].tsc);
-#endif
+
/* ensure they are zero */
memset(num, 0, sizeof(int) * MAX_CPU_NR);
memset(last_idx, 0, sizeof(int) * MAX_CPU_NR);
diff --git a/tools/tests/xen-access/xen-access.c
b/tools/tests/xen-access/xen-access.c
index 49195a8..b00c05a 100644
--- a/tools/tests/xen-access/xen-access.c
+++ b/tools/tests/xen-access/xen-access.c
@@ -219,10 +219,8 @@ int xenaccess_teardown(xc_interface *xch, xenaccess_t
*xenaccess)
}
xenaccess->xc_handle = NULL;
- if ( xenaccess->platform_info )
- free(xenaccess->platform_info);
- if ( xenaccess->domain_info )
- free(xenaccess->domain_info);
+ free(xenaccess->platform_info);
+ free(xenaccess->domain_info);
free(xenaccess);
return 0;
diff --git a/tools/xenbackendd/xenbackendd.c b/tools/xenbackendd/xenbackendd.c
index 5381a2a..f1eb1f5 100644
--- a/tools/xenbackendd/xenbackendd.c
+++ b/tools/xenbackendd/xenbackendd.c
@@ -291,8 +291,7 @@ main(int argc, char * const argv[])
switch(type) {
case DEVTYPE_VIF:
- if (s)
- free(s);
+ free(s);
snprintf(buf, sizeof(buf), "%s/script",
vec[XS_WATCH_PATH]);
s = xs_read(xs, XBT_NULL, buf, 0);
@@ -314,8 +313,7 @@ main(int argc, char * const argv[])
}
next2:
- if (s)
- free(s);
+ free(s);
free(sstate);
next1:
--
1.7.10.4
Matthew Daley
2013-Sep-29 03:26 UTC
[PATCH v2] remove unnecessary null pointer checks before frees
Oops, spatch removed an #if 0''d hunk from gtraceview.c. Here''s
a fixed version:
-- 8< --
Patch generated by the following semantic patch
(http://coccinelle.lip6.fr/):
@@
expression *P;
@@
- if(P) free(P);
+ free(P);
...and then by filtering through the following command:
filterdiff -p1 -x ''stubdom/*'' -x
''tools/firmware/*'' -x ''tools/qemu-*''
Signed-off-by: Matthew Daley <mattjd@gmail.com>
---
extras/mini-os/blkfront.c | 14 ++++++-------
extras/mini-os/console/xenbus.c | 8 +++----
extras/mini-os/fbfront.c | 38 +++++++++++++++++-----------------
extras/mini-os/kernel.c | 4 ++--
extras/mini-os/lib/sys.c | 3 +--
extras/mini-os/netfront.c | 18 ++++++++--------
extras/mini-os/pcifront.c | 14 ++++++-------
tools/blktap/drivers/blktapctrl.c | 6 ++----
tools/blktap/drivers/tapaio.c | 18 ++++++----------
tools/blktap/drivers/tapdisk.c | 9 +++-----
tools/blktap/lib/xenbus.c | 21 +++++++------------
tools/blktap/lib/xs_api.c | 3 +--
tools/blktap2/control/tap-ctl-list.c | 6 ++----
tools/blktap2/drivers/block-log.c | 3 +--
tools/flask/utils/loadpolicy.c | 3 +--
tools/libxc/xc_compression.c | 15 +++++---------
tools/libxc/xc_core_x86.c | 6 ++----
tools/libxc/xc_domain_save.c | 6 ++----
tools/libxc/xc_gnttab.c | 3 +--
tools/libxc/xc_offline_page.c | 9 +++-----
tools/libxl/libxl_event.c | 3 +--
tools/libxl/libxl_qmp.c | 6 ++----
tools/libxl/libxl_utils.c | 2 +-
tools/memshr/bidir-hash.c | 4 ++--
tools/misc/gtraceview.c | 3 +--
tools/tests/xen-access/xen-access.c | 6 ++----
tools/xenbackendd/xenbackendd.c | 6 ++----
27 files changed, 96 insertions(+), 141 deletions(-)
diff --git a/extras/mini-os/blkfront.c b/extras/mini-os/blkfront.c
index aead6bd..62a32c5 100644
--- a/extras/mini-os/blkfront.c
+++ b/extras/mini-os/blkfront.c
@@ -160,7 +160,7 @@ again:
err = xenbus_transaction_end(xbt, 0, &retry);
- if (err) free(err);
+ free(err);
if (retry) {
goto again;
printk("completing transaction\n");
@@ -271,7 +271,7 @@ void shutdown_blkfront(struct blkfront_dev *dev)
state = xenbus_read_integer(path);
while (err == NULL && state < XenbusStateClosing)
err = xenbus_wait_for_state_change(path, &state,
&dev->events);
- if (err) free(err);
+ free(err);
if ((err = xenbus_switch_state(XBT_NIL, nodename, XenbusStateClosed)) !=
NULL) {
printk("shutdown_blkfront: error changing state to %d: %s\n",
@@ -281,7 +281,7 @@ void shutdown_blkfront(struct blkfront_dev *dev)
state = xenbus_read_integer(path);
while (state < XenbusStateClosed) {
err = xenbus_wait_for_state_change(path, &state,
&dev->events);
- if (err) free(err);
+ free(err);
}
if ((err = xenbus_switch_state(XBT_NIL, nodename, XenbusStateInitialising))
!= NULL) {
@@ -294,16 +294,16 @@ void shutdown_blkfront(struct blkfront_dev *dev)
err = xenbus_wait_for_state_change(path, &state,
&dev->events);
close:
- if (err) free(err);
+ free(err);
err2 = xenbus_unwatch_path_token(XBT_NIL, path, path);
- if (err2) free(err2);
+ free(err2);
snprintf(nodename, sizeof(nodename), "%s/ring-ref",
dev->nodename);
err2 = xenbus_rm(XBT_NIL, nodename);
- if (err2) free(err2);
+ free(err2);
snprintf(nodename, sizeof(nodename), "%s/event-channel",
dev->nodename);
err2 = xenbus_rm(XBT_NIL, nodename);
- if (err2) free(err2);
+ free(err2);
if (!err)
free_blkfront(dev);
diff --git a/extras/mini-os/console/xenbus.c b/extras/mini-os/console/xenbus.c
index b317114..1c9a590 100644
--- a/extras/mini-os/console/xenbus.c
+++ b/extras/mini-os/console/xenbus.c
@@ -32,7 +32,7 @@ void free_consfront(struct consfront_dev *dev)
state = xenbus_read_integer(path);
while (err == NULL && state < XenbusStateClosing)
err = xenbus_wait_for_state_change(path, &state,
&dev->events);
- if (err) free(err);
+ free(err);
if ((err = xenbus_switch_state(XBT_NIL, nodename, XenbusStateClosed)) !=
NULL) {
printk("free_consfront: error changing state to %d: %s\n",
@@ -41,9 +41,9 @@ void free_consfront(struct consfront_dev *dev)
}
close:
- if (err) free(err);
+ free(err);
err = xenbus_unwatch_path_token(XBT_NIL, path, path);
- if (err) free(err);
+ free(err);
mask_evtchn(dev->evtchn);
unbind_evtchn(dev->evtchn);
@@ -134,7 +134,7 @@ again:
err = xenbus_transaction_end(xbt, 0, &retry);
- if (err) free(err);
+ free(err);
if (retry) {
goto again;
printk("completing transaction\n");
diff --git a/extras/mini-os/fbfront.c b/extras/mini-os/fbfront.c
index a276193..1e01513 100644
--- a/extras/mini-os/fbfront.c
+++ b/extras/mini-os/fbfront.c
@@ -131,7 +131,7 @@ again:
}
err = xenbus_transaction_end(xbt, 0, &retry);
- if (err) free(err);
+ free(err);
if (retry) {
goto again;
printk("completing transaction\n");
@@ -171,7 +171,7 @@ done:
err = xenbus_wait_for_state_change(path, &state,
&dev->events);
if (state != XenbusStateConnected) {
printk("backend not available, state=%d\n", state);
- if (err) free(err);
+ free(err);
err = xenbus_unwatch_path_token(XBT_NIL, path, path);
goto error;
}
@@ -256,7 +256,7 @@ void shutdown_kbdfront(struct kbdfront_dev *dev)
state = xenbus_read_integer(path);
while (err == NULL && state < XenbusStateClosing)
err = xenbus_wait_for_state_change(path, &state,
&dev->events);
- if (err) free(err);
+ free(err);
if ((err = xenbus_switch_state(XBT_NIL, nodename, XenbusStateClosed)) !=
NULL) {
printk("shutdown_kbdfront: error changing state to %d: %s\n",
@@ -266,7 +266,7 @@ void shutdown_kbdfront(struct kbdfront_dev *dev)
state = xenbus_read_integer(path);
while (state < XenbusStateClosed) {
err = xenbus_wait_for_state_change(path, &state,
&dev->events);
- if (err) free(err);
+ free(err);
}
if ((err = xenbus_switch_state(XBT_NIL, nodename, XenbusStateInitialising))
!= NULL) {
@@ -279,19 +279,19 @@ void shutdown_kbdfront(struct kbdfront_dev *dev)
err = xenbus_wait_for_state_change(path, &state, &dev->events);
close_kbdfront:
- if (err) free(err);
+ free(err);
err2 = xenbus_unwatch_path_token(XBT_NIL, path, path);
- if (err2) free(err2);
+ free(err2);
snprintf(nodename, sizeof(nodename), "%s/page-ref",
dev->nodename);
err2 = xenbus_rm(XBT_NIL, nodename);
- if (err2) free(err2);
+ free(err2);
snprintf(nodename, sizeof(nodename), "%s/event-channel",
dev->nodename);
err2 = xenbus_rm(XBT_NIL, nodename);
- if (err2) free(err2);
+ free(err2);
snprintf(nodename, sizeof(nodename), "%s/request-abs-pointer",
dev->nodename);
err2 = xenbus_rm(XBT_NIL, nodename);
- if (err2) free(err2);
+ free(err2);
if (!err)
free_kbdfront(dev);
@@ -498,7 +498,7 @@ again:
}
err = xenbus_transaction_end(xbt, 0, &retry);
- if (err) free(err);
+ free(err);
if (retry) {
goto again;
printk("completing transaction\n");
@@ -538,7 +538,7 @@ done:
err = xenbus_wait_for_state_change(path, &state,
&dev->events);
if (state != XenbusStateConnected) {
printk("backend not available, state=%d\n", state);
- if (err) free(err);
+ free(err);
err = xenbus_unwatch_path_token(XBT_NIL, path, path);
goto error;
}
@@ -654,7 +654,7 @@ void shutdown_fbfront(struct fbfront_dev *dev)
state = xenbus_read_integer(path);
while (err == NULL && state < XenbusStateClosing)
err = xenbus_wait_for_state_change(path, &state,
&dev->events);
- if (err) free(err);
+ free(err);
if ((err = xenbus_switch_state(XBT_NIL, nodename, XenbusStateClosed)) !=
NULL) {
printk("shutdown_fbfront: error changing state to %d: %s\n",
@@ -664,7 +664,7 @@ void shutdown_fbfront(struct fbfront_dev *dev)
state = xenbus_read_integer(path);
if (state < XenbusStateClosed) {
err = xenbus_wait_for_state_change(path, &state,
&dev->events);
- if (err) free(err);
+ free(err);
}
if ((err = xenbus_switch_state(XBT_NIL, nodename, XenbusStateInitialising))
!= NULL) {
@@ -677,22 +677,22 @@ void shutdown_fbfront(struct fbfront_dev *dev)
err = xenbus_wait_for_state_change(path, &state,
&dev->events);
close_fbfront:
- if (err) free(err);
+ free(err);
err2 = xenbus_unwatch_path_token(XBT_NIL, path, path);
- if (err2) free(err2);
+ free(err2);
snprintf(nodename, sizeof(nodename), "%s/page-ref",
dev->nodename);
err2 = xenbus_rm(XBT_NIL, nodename);
- if (err2) free(err2);
+ free(err2);
snprintf(nodename, sizeof(nodename), "%s/event-channel",
dev->nodename);
err2 = xenbus_rm(XBT_NIL, nodename);
- if (err2) free(err2);
+ free(err2);
snprintf(nodename, sizeof(nodename), "%s/protocol",
dev->nodename);
err2 = xenbus_rm(XBT_NIL, nodename);
- if (err2) free(err2);
+ free(err2);
snprintf(nodename, sizeof(nodename), "%s/feature-update",
dev->nodename);
err2 = xenbus_rm(XBT_NIL, nodename);
- if (err2) free(err2);
+ free(err2);
if (!err)
free_fbfront(dev);
diff --git a/extras/mini-os/kernel.c b/extras/mini-os/kernel.c
index 24fa25c..ea409f4 100644
--- a/extras/mini-os/kernel.c
+++ b/extras/mini-os/kernel.c
@@ -85,9 +85,9 @@ static void shutdown_thread(void *p)
xenbus_wait_for_watch(&events);
}
err = xenbus_unwatch_path_token(XBT_NIL, path, token);
- if (err) free(err);
+ free(err);
err = xenbus_write(XBT_NIL, path, "");
- if (err) free(err);
+ free(err);
printk("Shutting down (%s)\n", shutdown);
if (!strcmp(shutdown, "poweroff"))
diff --git a/extras/mini-os/lib/sys.c b/extras/mini-os/lib/sys.c
index cfbdc90..13e7e59 100644
--- a/extras/mini-os/lib/sys.c
+++ b/extras/mini-os/lib/sys.c
@@ -1156,8 +1156,7 @@ LWIP_STUB(int, getsockname, (int s, struct sockaddr *name,
socklen_t *namelen),
static char *syslog_ident;
void openlog(const char *ident, int option, int facility)
{
- if (syslog_ident)
- free(syslog_ident);
+ free(syslog_ident);
syslog_ident = strdup(ident);
}
diff --git a/extras/mini-os/netfront.c b/extras/mini-os/netfront.c
index 3a5be64..44c3995 100644
--- a/extras/mini-os/netfront.c
+++ b/extras/mini-os/netfront.c
@@ -412,7 +412,7 @@ again:
}
err = xenbus_transaction_end(xbt, 0, &retry);
- if (err) free(err);
+ free(err);
if (retry) {
goto again;
printk("completing transaction\n");
@@ -525,7 +525,7 @@ void shutdown_netfront(struct netfront_dev *dev)
state = xenbus_read_integer(path);
while (err == NULL && state < XenbusStateClosing)
err = xenbus_wait_for_state_change(path, &state,
&dev->events);
- if (err) free(err);
+ free(err);
if ((err = xenbus_switch_state(XBT_NIL, nodename, XenbusStateClosed)) !=
NULL) {
printk("shutdown_netfront: error changing state to %d: %s\n",
@@ -535,7 +535,7 @@ void shutdown_netfront(struct netfront_dev *dev)
state = xenbus_read_integer(path);
while (state < XenbusStateClosed) {
err = xenbus_wait_for_state_change(path, &state,
&dev->events);
- if (err) free(err);
+ free(err);
}
if ((err = xenbus_switch_state(XBT_NIL, nodename, XenbusStateInitialising))
!= NULL) {
@@ -548,22 +548,22 @@ void shutdown_netfront(struct netfront_dev *dev)
err = xenbus_wait_for_state_change(path, &state,
&dev->events);
close:
- if (err) free(err);
+ free(err);
err2 = xenbus_unwatch_path_token(XBT_NIL, path, path);
- if (err2) free(err2);
+ free(err2);
snprintf(nodename, sizeof(nodename), "%s/tx-ring-ref",
dev->nodename);
err2 = xenbus_rm(XBT_NIL, nodename);
- if (err2) free(err2);
+ free(err2);
snprintf(nodename, sizeof(nodename), "%s/rx-ring-ref",
dev->nodename);
err2 = xenbus_rm(XBT_NIL, nodename);
- if (err2) free(err2);
+ free(err2);
snprintf(nodename, sizeof(nodename), "%s/event-channel",
dev->nodename);
err2 = xenbus_rm(XBT_NIL, nodename);
- if (err2) free(err2);
+ free(err2);
snprintf(nodename, sizeof(nodename), "%s/request-rx-copy",
dev->nodename);
err2 = xenbus_rm(XBT_NIL, nodename);
- if (err2) free(err2);
+ free(err2);
if (!err)
free_netfront(dev);
diff --git a/extras/mini-os/pcifront.c b/extras/mini-os/pcifront.c
index 7838021..16a4b49 100644
--- a/extras/mini-os/pcifront.c
+++ b/extras/mini-os/pcifront.c
@@ -212,7 +212,7 @@ again:
}
err = xenbus_transaction_end(xbt, 0, &retry);
- if (err) free(err);
+ free(err);
if (retry) {
goto again;
printk("completing transaction\n");
@@ -251,7 +251,7 @@ done:
err = xenbus_wait_for_state_change(path, &state,
&dev->events);
if (state != XenbusStateConnected) {
printk("backend not avalable, state=%d\n", state);
- if (err) free(err);
+ free(err);
err = xenbus_unwatch_path_token(XBT_NIL, path, path);
goto error;
}
@@ -342,7 +342,7 @@ void shutdown_pcifront(struct pcifront_dev *dev)
state = xenbus_read_integer(path);
while (err == NULL && state < XenbusStateClosing)
err = xenbus_wait_for_state_change(path, &state,
&dev->events);
- if (err) free(err);
+ free(err);
if ((err = xenbus_switch_state(XBT_NIL, nodename, XenbusStateClosed)) !=
NULL) {
printk("shutdown_pcifront: error changing state to %d: %s\n",
@@ -365,16 +365,16 @@ void shutdown_pcifront(struct pcifront_dev *dev)
err = xenbus_wait_for_state_change(path, &state,
&dev->events);
close_pcifront:
- if (err) free(err);
+ free(err);
err2 = xenbus_unwatch_path_token(XBT_NIL, path, path);
- if (err2) free(err2);
+ free(err2);
snprintf(nodename, sizeof(nodename), "%s/info-ref",
dev->nodename);
err2 = xenbus_rm(XBT_NIL, nodename);
- if (err2) free(err2);
+ free(err2);
snprintf(nodename, sizeof(nodename), "%s/event-channel",
dev->nodename);
err2 = xenbus_rm(XBT_NIL, nodename);
- if (err2) free(err2);
+ free(err2);
if (!err)
free_pcifront(dev);
diff --git a/tools/blktap/drivers/blktapctrl.c
b/tools/blktap/drivers/blktapctrl.c
index 0a8b880..0022264 100644
--- a/tools/blktap/drivers/blktapctrl.c
+++ b/tools/blktap/drivers/blktapctrl.c
@@ -642,11 +642,9 @@ static int connect_tapdisk(blkif_t *blkif, int minor)
ret = 0;
fail:
- if (rdctldev)
- free(rdctldev);
+ free(rdctldev);
- if (wrctldev)
- free(wrctldev);
+ free(wrctldev);
return ret;
}
diff --git a/tools/blktap/drivers/tapaio.c b/tools/blktap/drivers/tapaio.c
index 140c44a..ae26577 100644
--- a/tools/blktap/drivers/tapaio.c
+++ b/tools/blktap/drivers/tapaio.c
@@ -244,18 +244,12 @@ fail:
void tap_aio_free(tap_aio_context_t *ctx)
{
- if (ctx->sector_lock)
- free(ctx->sector_lock);
- if (ctx->iocb_list)
- free(ctx->iocb_list);
- if (ctx->pending_aio)
- free(ctx->pending_aio);
- if (ctx->aio_events)
- free(ctx->aio_events);
- if (ctx->iocb_free)
- free(ctx->iocb_free);
- if (ctx->iocb_queue)
- free(ctx->iocb_queue);
+ free(ctx->sector_lock);
+ free(ctx->iocb_list);
+ free(ctx->pending_aio);
+ free(ctx->aio_events);
+ free(ctx->iocb_free);
+ free(ctx->iocb_queue);
}
/*TODO: Fix sector span!*/
diff --git a/tools/blktap/drivers/tapdisk.c b/tools/blktap/drivers/tapdisk.c
index 19cd777..a6e8a7c 100644
--- a/tools/blktap/drivers/tapdisk.c
+++ b/tools/blktap/drivers/tapdisk.c
@@ -82,10 +82,8 @@ static void daemonize(void)
static void free_driver(struct disk_driver *d)
{
- if (d->name)
- free(d->name);
- if (d->private)
- free(d->private);
+ free(d->name);
+ free(d->private);
free(d);
}
@@ -354,8 +352,7 @@ static int open_disk(struct td_state *s,
fail:
DPRINTF("failed opening disk\n");
- if (id.name)
- free(id.name);
+ free(id.name);
d = s->disks;
while (d) {
struct disk_driver *tmp = d->next;
diff --git a/tools/blktap/lib/xenbus.c b/tools/blktap/lib/xenbus.c
index 948eb02..9edbf5a 100644
--- a/tools/blktap/lib/xenbus.c
+++ b/tools/blktap/lib/xenbus.c
@@ -160,10 +160,8 @@ static int backend_remove(struct xs_handle *h, struct
backend_info *be)
DPRINTF("Freeing blkif dev [%d]\n",be->blkif->devnum);
free_blkif(be->blkif);
}
- if (be->frontpath)
- free(be->frontpath);
- if (be->backpath)
- free(be->backpath);
+ free(be->frontpath);
+ free(be->backpath);
free(be);
return 0;
}
@@ -435,8 +433,7 @@ fail:
backend_remove(h, be);
}
close:
- if (path)
- free(path);
+ free(path);
return;
}
@@ -501,8 +498,7 @@ static void ueblktap_probe(struct xs_handle *h, struct
xenbus_watch *w,
if ( (be != NULL) && (be->blkif != NULL) )
backend_remove(h, be);
else goto free_be;
- if (bepath)
- free(bepath);
+ free(bepath);
return;
}
@@ -522,12 +518,9 @@ static void ueblktap_probe(struct xs_handle *h, struct
xenbus_watch *w,
return;
free_be:
- if (frontend)
- free(frontend);
- if (bepath)
- free(bepath);
- if (be)
- free(be);
+ free(frontend);
+ free(bepath);
+ free(be);
}
/**
diff --git a/tools/blktap/lib/xs_api.c b/tools/blktap/lib/xs_api.c
index 4648432..1624cb9 100644
--- a/tools/blktap/lib/xs_api.c
+++ b/tools/blktap/lib/xs_api.c
@@ -204,8 +204,7 @@ char *get_dom_domid(struct xs_handle *h)
}
done:
xs_transaction_end(h, xth, 0);
- if (e)
- free(e);
+ free(e);
return domid;
}
diff --git a/tools/blktap2/control/tap-ctl-list.c
b/tools/blktap2/control/tap-ctl-list.c
index c91f6f4..170735e 100644
--- a/tools/blktap2/control/tap-ctl-list.c
+++ b/tools/blktap2/control/tap-ctl-list.c
@@ -219,8 +219,7 @@ out:
return err ? : n_minors;
fail:
- if (minorv)
- free(minorv);
+ free(minorv);
goto out;
}
@@ -302,8 +301,7 @@ out:
return err ? : n_taps;
fail:
- if (tapv)
- free(tapv);
+ free(tapv);
goto out;
}
diff --git a/tools/blktap2/drivers/block-log.c
b/tools/blktap2/drivers/block-log.c
index 5330cdc..21c1946 100644
--- a/tools/blktap2/drivers/block-log.c
+++ b/tools/blktap2/drivers/block-log.c
@@ -110,8 +110,7 @@ static int writelog_create(struct tdlog_state *s)
static int writelog_free(struct tdlog_state *s)
{
- if (s->writelog)
- free(s->writelog);
+ free(s->writelog);
return 0;
}
diff --git a/tools/flask/utils/loadpolicy.c b/tools/flask/utils/loadpolicy.c
index f347b97..2edcf97 100644
--- a/tools/flask/utils/loadpolicy.c
+++ b/tools/flask/utils/loadpolicy.c
@@ -108,8 +108,7 @@ int main (int argCnt, const char *args[])
}
done:
- if ( polMemCp )
- free(polMemCp);
+ free(polMemCp);
if ( polMem )
{
ret = munmap(polMem, info.st_size);
diff --git a/tools/libxc/xc_compression.c b/tools/libxc/xc_compression.c
index 89f1be7..8f0b89d 100644
--- a/tools/libxc/xc_compression.c
+++ b/tools/libxc/xc_compression.c
@@ -457,16 +457,11 @@ void xc_compression_free_context(xc_interface *xch,
comp_ctx *ctx)
{
if (!ctx) return;
- if (ctx->inputbuf)
- free(ctx->inputbuf);
- if (ctx->sendbuf_pfns)
- free(ctx->sendbuf_pfns);
- if (ctx->cache_base)
- free(ctx->cache_base);
- if (ctx->pfn2cache)
- free(ctx->pfn2cache);
- if (ctx->cache)
- free(ctx->cache);
+ free(ctx->inputbuf);
+ free(ctx->sendbuf_pfns);
+ free(ctx->cache_base);
+ free(ctx->pfn2cache);
+ free(ctx->cache);
free(ctx);
}
diff --git a/tools/libxc/xc_core_x86.c b/tools/libxc/xc_core_x86.c
index a9681f7..e328dcf 100644
--- a/tools/libxc/xc_core_x86.c
+++ b/tools/libxc/xc_core_x86.c
@@ -180,11 +180,9 @@ out:
if ( live_p2m_frame_list )
munmap(live_p2m_frame_list, P2M_FLL_ENTRIES * PAGE_SIZE);
- if ( p2m_frame_list_list )
- free(p2m_frame_list_list);
+ free(p2m_frame_list_list);
- if ( p2m_frame_list )
- free(p2m_frame_list);
+ free(p2m_frame_list);
errno = err;
return ret;
diff --git a/tools/libxc/xc_domain_save.c b/tools/libxc/xc_domain_save.c
index fbc15e9..673d1e9 100644
--- a/tools/libxc/xc_domain_save.c
+++ b/tools/libxc/xc_domain_save.c
@@ -773,11 +773,9 @@ static xen_pfn_t *map_and_save_p2m_table(xc_interface *xch,
if ( live_p2m_frame_list )
munmap(live_p2m_frame_list, P2M_FLL_ENTRIES * PAGE_SIZE);
- if ( p2m_frame_list_list )
- free(p2m_frame_list_list);
+ free(p2m_frame_list_list);
- if ( p2m_frame_list )
- free(p2m_frame_list);
+ free(p2m_frame_list);
return success ? p2m : NULL;
}
diff --git a/tools/libxc/xc_gnttab.c b/tools/libxc/xc_gnttab.c
index a8cc381..79dab40 100644
--- a/tools/libxc/xc_gnttab.c
+++ b/tools/libxc/xc_gnttab.c
@@ -124,8 +124,7 @@ static void *_gnttab_map_table(xc_interface *xch, int domid,
int *gnt_num)
err:
if ( frame_list )
xc_hypercall_buffer_free(xch, frame_list);
- if ( pfn_list )
- free(pfn_list);
+ free(pfn_list);
return gnt;
}
diff --git a/tools/libxc/xc_offline_page.c b/tools/libxc/xc_offline_page.c
index 8195efb..0b4cdf9 100644
--- a/tools/libxc/xc_offline_page.c
+++ b/tools/libxc/xc_offline_page.c
@@ -626,14 +626,11 @@ failed:
}
}
- if (mmu)
- free(mmu);
+ free(mmu);
- if (old_ptes.entries)
- free(old_ptes.entries);
+ free(old_ptes.entries);
- if (backup)
- free(backup);
+ free(backup);
if (gnttab_v1)
munmap(gnttab_v1, gnt_num / (PAGE_SIZE/sizeof(grant_entry_v1_t)));
diff --git a/tools/libxl/libxl_event.c b/tools/libxl/libxl_event.c
index fd2f280..73b7e63 100644
--- a/tools/libxl/libxl_event.c
+++ b/tools/libxl/libxl_event.c
@@ -578,8 +578,7 @@ int libxl__ev_xswatch_register(libxl__gc *gc,
libxl__ev_xswatch *w,
out_rc:
if (use)
LIBXL_SLIST_INSERT_HEAD(&CTX->watch_freeslots, use, empty);
- if (path_copy)
- free(path_copy);
+ free(path_copy);
CTX_UNLOCK;
return rc;
}
diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
index f681f3a..7e825ee 100644
--- a/tools/libxl/libxl_qmp.c
+++ b/tools/libxl/libxl_qmp.c
@@ -406,12 +406,10 @@ static void qmp_close(libxl__qmp_handler *qmp)
close(qmp->qmp_fd);
LIBXL_STAILQ_FOREACH(pp, &qmp->callback_list, next) {
- if (tmp)
- free(tmp);
+ free(tmp);
tmp = pp;
}
- if (tmp)
- free(tmp);
+ free(tmp);
}
static int qmp_next(libxl__gc *gc, libxl__qmp_handler *qmp)
diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
index 8e567a8..664a730 100644
--- a/tools/libxl/libxl_utils.c
+++ b/tools/libxl/libxl_utils.c
@@ -355,7 +355,7 @@ int libxl_read_file_contents(libxl_ctx *ctx, const char
*filename,
e = errno;
assert(e != ENOENT);
if (f) fclose(f);
- if (data) free(data);
+ free(data);
return e;
}
diff --git a/tools/memshr/bidir-hash.c b/tools/memshr/bidir-hash.c
index bed8179..3d34637 100644
--- a/tools/memshr/bidir-hash.c
+++ b/tools/memshr/bidir-hash.c
@@ -208,8 +208,8 @@ static void free_buckets(struct __hash *h,
struct bucket *buckets,
struct bucket_lock *bucket_locks)
{
- if(buckets) free(buckets);
- if(bucket_locks) free(bucket_locks);
+ free(buckets);
+ free(bucket_locks);
}
static int max_entries(struct __hash *h)
diff --git a/tools/misc/gtraceview.c b/tools/misc/gtraceview.c
index d8b4589..cf9287c 100644
--- a/tools/misc/gtraceview.c
+++ b/tools/misc/gtraceview.c
@@ -959,8 +959,7 @@ int time_mode_rebuild(uint64_t start_time, uint64_t
time_scale)
state = malloc(sizeof(struct state) * number);
if (!state)
return 1;
- if (this->state)
- free(this->state);
+ free(this->state);
this->state = state;
this->width = 9;
this->row = 0;
diff --git a/tools/tests/xen-access/xen-access.c
b/tools/tests/xen-access/xen-access.c
index 49195a8..b00c05a 100644
--- a/tools/tests/xen-access/xen-access.c
+++ b/tools/tests/xen-access/xen-access.c
@@ -219,10 +219,8 @@ int xenaccess_teardown(xc_interface *xch, xenaccess_t
*xenaccess)
}
xenaccess->xc_handle = NULL;
- if ( xenaccess->platform_info )
- free(xenaccess->platform_info);
- if ( xenaccess->domain_info )
- free(xenaccess->domain_info);
+ free(xenaccess->platform_info);
+ free(xenaccess->domain_info);
free(xenaccess);
return 0;
diff --git a/tools/xenbackendd/xenbackendd.c b/tools/xenbackendd/xenbackendd.c
index 5381a2a..f1eb1f5 100644
--- a/tools/xenbackendd/xenbackendd.c
+++ b/tools/xenbackendd/xenbackendd.c
@@ -291,8 +291,7 @@ main(int argc, char * const argv[])
switch(type) {
case DEVTYPE_VIF:
- if (s)
- free(s);
+ free(s);
snprintf(buf, sizeof(buf), "%s/script",
vec[XS_WATCH_PATH]);
s = xs_read(xs, XBT_NULL, buf, 0);
@@ -314,8 +313,7 @@ main(int argc, char * const argv[])
}
next2:
- if (s)
- free(s);
+ free(s);
free(sstate);
next1:
--
1.7.10.4
Matthew Daley
2013-Sep-29 05:24 UTC
[PATCH v2] libxl: only put poller if already gotten in libxl_event_wait
Coverity-ID: 1055292
Signed-off-by: Matthew Daley <mattjd@gmail.com>
---
tools/libxl/libxl_event.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tools/libxl/libxl_event.c b/tools/libxl/libxl_event.c
index 13bdd12..0b1a7d8 100644
--- a/tools/libxl/libxl_event.c
+++ b/tools/libxl/libxl_event.c
@@ -1475,7 +1475,8 @@ int libxl_event_wait(libxl_ctx *ctx, libxl_event
**event_r,
}
out:
- libxl__poller_put(ctx, poller);
+ if (poller)
+ libxl__poller_put(ctx, poller);
CTX_UNLOCK;
EGC_FREE;
--
1.7.10.4
Matthew Daley
2013-Sep-29 05:47 UTC
[PATCH v2] libxl: correctly handle libxl_get_cpu_topology failure in libxl_{cpu, node}map_to_{node, cpu}map
Initialize nr_cpus to 0 so that if it is unchanged by a failing
libxl_get_cpu_topology, libxl_cputopology_list_free still works OK
afterward.
Coverity-ID: 1055294
Coverity-ID: 1055295
Signed-off-by: Matthew Daley <mattjd@gmail.com>
---
tools/libxl/libxl_utils.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
index 1927383..bb5ae31 100644
--- a/tools/libxl/libxl_utils.c
+++ b/tools/libxl/libxl_utils.c
@@ -650,7 +650,7 @@ int libxl_nodemap_to_cpumap(libxl_ctx *ctx,
libxl_bitmap *cpumap)
{
libxl_cputopology *tinfo = NULL;
- int nr_cpus, i, rc = 0;
+ int nr_cpus = 0, i, rc = 0;
tinfo = libxl_get_cpu_topology(ctx, &nr_cpus);
if (tinfo == NULL) {
@@ -673,7 +673,7 @@ int libxl_cpumap_to_nodemap(libxl_ctx *ctx,
libxl_bitmap *nodemap)
{
libxl_cputopology *tinfo = NULL;
- int nr_cpus, i, rc = 0;
+ int nr_cpus = 0, i, rc = 0;
tinfo = libxl_get_cpu_topology(ctx, &nr_cpus);
if (tinfo == NULL) {
--
1.7.10.4
Dario Faggioli
2013-Sep-30 10:29 UTC
Re: [PATCH v2] libxl: correctly handle libxl_get_cpu_topology failure in libxl_{cpu, node}map_to_{node, cpu}map
On dom, 2013-09-29 at 18:47 +1300, Matthew Daley wrote:> Initialize nr_cpus to 0 so that if it is unchanged by a failing > libxl_get_cpu_topology, libxl_cputopology_list_free still works OK > afterward. > > Coverity-ID: 1055294 > Coverity-ID: 1055295 > Signed-off-by: Matthew Daley <mattjd@gmail.com> >Acked-by: Dario Faggioli <dario.faggioli@citrix.com> And sorry I did not get to do it earlier! :-P Thanks and Regards, Dario -- <<This happens because I choose it to happen!>> (Raistlin Majere) ----------------------------------------------------------------- Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK) _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Samuel Thibault
2013-Sep-30 21:15 UTC
Re: [PATCH v2] remove unnecessary null pointer checks before frees
Matthew Daley, le Sun 29 Sep 2013 16:26:58 +1300, a écrit :> Oops, spatch removed an #if 0''d hunk from gtraceview.c. Here''s a fixed version: > > -- 8< -- > > Patch generated by the following semantic patch > (http://coccinelle.lip6.fr/): > > @@ > expression *P; > @@ > > - if(P) free(P); > + free(P); > > ...and then by filtering through the following command: > > filterdiff -p1 -x ''stubdom/*'' -x ''tools/firmware/*'' -x ''tools/qemu-*'' > > Signed-off-by: Matthew Daley <mattjd@gmail.com>Acked-by: Samuel Thibault <samuel.thibault@ens-lyon.org>> --- > extras/mini-os/blkfront.c | 14 ++++++------- > extras/mini-os/console/xenbus.c | 8 +++---- > extras/mini-os/fbfront.c | 38 +++++++++++++++++----------------- > extras/mini-os/kernel.c | 4 ++-- > extras/mini-os/lib/sys.c | 3 +-- > extras/mini-os/netfront.c | 18 ++++++++-------- > extras/mini-os/pcifront.c | 14 ++++++------- > tools/blktap/drivers/blktapctrl.c | 6 ++---- > tools/blktap/drivers/tapaio.c | 18 ++++++---------- > tools/blktap/drivers/tapdisk.c | 9 +++----- > tools/blktap/lib/xenbus.c | 21 +++++++------------ > tools/blktap/lib/xs_api.c | 3 +-- > tools/blktap2/control/tap-ctl-list.c | 6 ++---- > tools/blktap2/drivers/block-log.c | 3 +-- > tools/flask/utils/loadpolicy.c | 3 +-- > tools/libxc/xc_compression.c | 15 +++++--------- > tools/libxc/xc_core_x86.c | 6 ++---- > tools/libxc/xc_domain_save.c | 6 ++---- > tools/libxc/xc_gnttab.c | 3 +-- > tools/libxc/xc_offline_page.c | 9 +++----- > tools/libxl/libxl_event.c | 3 +-- > tools/libxl/libxl_qmp.c | 6 ++---- > tools/libxl/libxl_utils.c | 2 +- > tools/memshr/bidir-hash.c | 4 ++-- > tools/misc/gtraceview.c | 3 +-- > tools/tests/xen-access/xen-access.c | 6 ++---- > tools/xenbackendd/xenbackendd.c | 6 ++---- > 27 files changed, 96 insertions(+), 141 deletions(-) > > diff --git a/extras/mini-os/blkfront.c b/extras/mini-os/blkfront.c > index aead6bd..62a32c5 100644 > --- a/extras/mini-os/blkfront.c > +++ b/extras/mini-os/blkfront.c > @@ -160,7 +160,7 @@ again: > > > err = xenbus_transaction_end(xbt, 0, &retry); > - if (err) free(err); > + free(err); > if (retry) { > goto again; > printk("completing transaction\n"); > @@ -271,7 +271,7 @@ void shutdown_blkfront(struct blkfront_dev *dev) > state = xenbus_read_integer(path); > while (err == NULL && state < XenbusStateClosing) > err = xenbus_wait_for_state_change(path, &state, &dev->events); > - if (err) free(err); > + free(err); > > if ((err = xenbus_switch_state(XBT_NIL, nodename, XenbusStateClosed)) != NULL) { > printk("shutdown_blkfront: error changing state to %d: %s\n", > @@ -281,7 +281,7 @@ void shutdown_blkfront(struct blkfront_dev *dev) > state = xenbus_read_integer(path); > while (state < XenbusStateClosed) { > err = xenbus_wait_for_state_change(path, &state, &dev->events); > - if (err) free(err); > + free(err); > } > > if ((err = xenbus_switch_state(XBT_NIL, nodename, XenbusStateInitialising)) != NULL) { > @@ -294,16 +294,16 @@ void shutdown_blkfront(struct blkfront_dev *dev) > err = xenbus_wait_for_state_change(path, &state, &dev->events); > > close: > - if (err) free(err); > + free(err); > err2 = xenbus_unwatch_path_token(XBT_NIL, path, path); > - if (err2) free(err2); > + free(err2); > > snprintf(nodename, sizeof(nodename), "%s/ring-ref", dev->nodename); > err2 = xenbus_rm(XBT_NIL, nodename); > - if (err2) free(err2); > + free(err2); > snprintf(nodename, sizeof(nodename), "%s/event-channel", dev->nodename); > err2 = xenbus_rm(XBT_NIL, nodename); > - if (err2) free(err2); > + free(err2); > > if (!err) > free_blkfront(dev); > diff --git a/extras/mini-os/console/xenbus.c b/extras/mini-os/console/xenbus.c > index b317114..1c9a590 100644 > --- a/extras/mini-os/console/xenbus.c > +++ b/extras/mini-os/console/xenbus.c > @@ -32,7 +32,7 @@ void free_consfront(struct consfront_dev *dev) > state = xenbus_read_integer(path); > while (err == NULL && state < XenbusStateClosing) > err = xenbus_wait_for_state_change(path, &state, &dev->events); > - if (err) free(err); > + free(err); > > if ((err = xenbus_switch_state(XBT_NIL, nodename, XenbusStateClosed)) != NULL) { > printk("free_consfront: error changing state to %d: %s\n", > @@ -41,9 +41,9 @@ void free_consfront(struct consfront_dev *dev) > } > > close: > - if (err) free(err); > + free(err); > err = xenbus_unwatch_path_token(XBT_NIL, path, path); > - if (err) free(err); > + free(err); > > mask_evtchn(dev->evtchn); > unbind_evtchn(dev->evtchn); > @@ -134,7 +134,7 @@ again: > > > err = xenbus_transaction_end(xbt, 0, &retry); > - if (err) free(err); > + free(err); > if (retry) { > goto again; > printk("completing transaction\n"); > diff --git a/extras/mini-os/fbfront.c b/extras/mini-os/fbfront.c > index a276193..1e01513 100644 > --- a/extras/mini-os/fbfront.c > +++ b/extras/mini-os/fbfront.c > @@ -131,7 +131,7 @@ again: > } > > err = xenbus_transaction_end(xbt, 0, &retry); > - if (err) free(err); > + free(err); > if (retry) { > goto again; > printk("completing transaction\n"); > @@ -171,7 +171,7 @@ done: > err = xenbus_wait_for_state_change(path, &state, &dev->events); > if (state != XenbusStateConnected) { > printk("backend not available, state=%d\n", state); > - if (err) free(err); > + free(err); > err = xenbus_unwatch_path_token(XBT_NIL, path, path); > goto error; > } > @@ -256,7 +256,7 @@ void shutdown_kbdfront(struct kbdfront_dev *dev) > state = xenbus_read_integer(path); > while (err == NULL && state < XenbusStateClosing) > err = xenbus_wait_for_state_change(path, &state, &dev->events); > - if (err) free(err); > + free(err); > > if ((err = xenbus_switch_state(XBT_NIL, nodename, XenbusStateClosed)) != NULL) { > printk("shutdown_kbdfront: error changing state to %d: %s\n", > @@ -266,7 +266,7 @@ void shutdown_kbdfront(struct kbdfront_dev *dev) > state = xenbus_read_integer(path); > while (state < XenbusStateClosed) { > err = xenbus_wait_for_state_change(path, &state, &dev->events); > - if (err) free(err); > + free(err); > } > > if ((err = xenbus_switch_state(XBT_NIL, nodename, XenbusStateInitialising)) != NULL) { > @@ -279,19 +279,19 @@ void shutdown_kbdfront(struct kbdfront_dev *dev) > err = xenbus_wait_for_state_change(path, &state, &dev->events); > > close_kbdfront: > - if (err) free(err); > + free(err); > err2 = xenbus_unwatch_path_token(XBT_NIL, path, path); > - if (err2) free(err2); > + free(err2); > > snprintf(nodename, sizeof(nodename), "%s/page-ref", dev->nodename); > err2 = xenbus_rm(XBT_NIL, nodename); > - if (err2) free(err2); > + free(err2); > snprintf(nodename, sizeof(nodename), "%s/event-channel", dev->nodename); > err2 = xenbus_rm(XBT_NIL, nodename); > - if (err2) free(err2); > + free(err2); > snprintf(nodename, sizeof(nodename), "%s/request-abs-pointer", dev->nodename); > err2 = xenbus_rm(XBT_NIL, nodename); > - if (err2) free(err2); > + free(err2); > > if (!err) > free_kbdfront(dev); > @@ -498,7 +498,7 @@ again: > } > > err = xenbus_transaction_end(xbt, 0, &retry); > - if (err) free(err); > + free(err); > if (retry) { > goto again; > printk("completing transaction\n"); > @@ -538,7 +538,7 @@ done: > err = xenbus_wait_for_state_change(path, &state, &dev->events); > if (state != XenbusStateConnected) { > printk("backend not available, state=%d\n", state); > - if (err) free(err); > + free(err); > err = xenbus_unwatch_path_token(XBT_NIL, path, path); > goto error; > } > @@ -654,7 +654,7 @@ void shutdown_fbfront(struct fbfront_dev *dev) > state = xenbus_read_integer(path); > while (err == NULL && state < XenbusStateClosing) > err = xenbus_wait_for_state_change(path, &state, &dev->events); > - if (err) free(err); > + free(err); > > if ((err = xenbus_switch_state(XBT_NIL, nodename, XenbusStateClosed)) != NULL) { > printk("shutdown_fbfront: error changing state to %d: %s\n", > @@ -664,7 +664,7 @@ void shutdown_fbfront(struct fbfront_dev *dev) > state = xenbus_read_integer(path); > if (state < XenbusStateClosed) { > err = xenbus_wait_for_state_change(path, &state, &dev->events); > - if (err) free(err); > + free(err); > } > > if ((err = xenbus_switch_state(XBT_NIL, nodename, XenbusStateInitialising)) != NULL) { > @@ -677,22 +677,22 @@ void shutdown_fbfront(struct fbfront_dev *dev) > err = xenbus_wait_for_state_change(path, &state, &dev->events); > > close_fbfront: > - if (err) free(err); > + free(err); > err2 = xenbus_unwatch_path_token(XBT_NIL, path, path); > - if (err2) free(err2); > + free(err2); > > snprintf(nodename, sizeof(nodename), "%s/page-ref", dev->nodename); > err2 = xenbus_rm(XBT_NIL, nodename); > - if (err2) free(err2); > + free(err2); > snprintf(nodename, sizeof(nodename), "%s/event-channel", dev->nodename); > err2 = xenbus_rm(XBT_NIL, nodename); > - if (err2) free(err2); > + free(err2); > snprintf(nodename, sizeof(nodename), "%s/protocol", dev->nodename); > err2 = xenbus_rm(XBT_NIL, nodename); > - if (err2) free(err2); > + free(err2); > snprintf(nodename, sizeof(nodename), "%s/feature-update", dev->nodename); > err2 = xenbus_rm(XBT_NIL, nodename); > - if (err2) free(err2); > + free(err2); > > if (!err) > free_fbfront(dev); > diff --git a/extras/mini-os/kernel.c b/extras/mini-os/kernel.c > index 24fa25c..ea409f4 100644 > --- a/extras/mini-os/kernel.c > +++ b/extras/mini-os/kernel.c > @@ -85,9 +85,9 @@ static void shutdown_thread(void *p) > xenbus_wait_for_watch(&events); > } > err = xenbus_unwatch_path_token(XBT_NIL, path, token); > - if (err) free(err); > + free(err); > err = xenbus_write(XBT_NIL, path, ""); > - if (err) free(err); > + free(err); > printk("Shutting down (%s)\n", shutdown); > > if (!strcmp(shutdown, "poweroff")) > diff --git a/extras/mini-os/lib/sys.c b/extras/mini-os/lib/sys.c > index cfbdc90..13e7e59 100644 > --- a/extras/mini-os/lib/sys.c > +++ b/extras/mini-os/lib/sys.c > @@ -1156,8 +1156,7 @@ LWIP_STUB(int, getsockname, (int s, struct sockaddr *name, socklen_t *namelen), > static char *syslog_ident; > void openlog(const char *ident, int option, int facility) > { > - if (syslog_ident) > - free(syslog_ident); > + free(syslog_ident); > syslog_ident = strdup(ident); > } > > diff --git a/extras/mini-os/netfront.c b/extras/mini-os/netfront.c > index 3a5be64..44c3995 100644 > --- a/extras/mini-os/netfront.c > +++ b/extras/mini-os/netfront.c > @@ -412,7 +412,7 @@ again: > } > > err = xenbus_transaction_end(xbt, 0, &retry); > - if (err) free(err); > + free(err); > if (retry) { > goto again; > printk("completing transaction\n"); > @@ -525,7 +525,7 @@ void shutdown_netfront(struct netfront_dev *dev) > state = xenbus_read_integer(path); > while (err == NULL && state < XenbusStateClosing) > err = xenbus_wait_for_state_change(path, &state, &dev->events); > - if (err) free(err); > + free(err); > > if ((err = xenbus_switch_state(XBT_NIL, nodename, XenbusStateClosed)) != NULL) { > printk("shutdown_netfront: error changing state to %d: %s\n", > @@ -535,7 +535,7 @@ void shutdown_netfront(struct netfront_dev *dev) > state = xenbus_read_integer(path); > while (state < XenbusStateClosed) { > err = xenbus_wait_for_state_change(path, &state, &dev->events); > - if (err) free(err); > + free(err); > } > > if ((err = xenbus_switch_state(XBT_NIL, nodename, XenbusStateInitialising)) != NULL) { > @@ -548,22 +548,22 @@ void shutdown_netfront(struct netfront_dev *dev) > err = xenbus_wait_for_state_change(path, &state, &dev->events); > > close: > - if (err) free(err); > + free(err); > err2 = xenbus_unwatch_path_token(XBT_NIL, path, path); > - if (err2) free(err2); > + free(err2); > > snprintf(nodename, sizeof(nodename), "%s/tx-ring-ref", dev->nodename); > err2 = xenbus_rm(XBT_NIL, nodename); > - if (err2) free(err2); > + free(err2); > snprintf(nodename, sizeof(nodename), "%s/rx-ring-ref", dev->nodename); > err2 = xenbus_rm(XBT_NIL, nodename); > - if (err2) free(err2); > + free(err2); > snprintf(nodename, sizeof(nodename), "%s/event-channel", dev->nodename); > err2 = xenbus_rm(XBT_NIL, nodename); > - if (err2) free(err2); > + free(err2); > snprintf(nodename, sizeof(nodename), "%s/request-rx-copy", dev->nodename); > err2 = xenbus_rm(XBT_NIL, nodename); > - if (err2) free(err2); > + free(err2); > > if (!err) > free_netfront(dev); > diff --git a/extras/mini-os/pcifront.c b/extras/mini-os/pcifront.c > index 7838021..16a4b49 100644 > --- a/extras/mini-os/pcifront.c > +++ b/extras/mini-os/pcifront.c > @@ -212,7 +212,7 @@ again: > } > > err = xenbus_transaction_end(xbt, 0, &retry); > - if (err) free(err); > + free(err); > if (retry) { > goto again; > printk("completing transaction\n"); > @@ -251,7 +251,7 @@ done: > err = xenbus_wait_for_state_change(path, &state, &dev->events); > if (state != XenbusStateConnected) { > printk("backend not avalable, state=%d\n", state); > - if (err) free(err); > + free(err); > err = xenbus_unwatch_path_token(XBT_NIL, path, path); > goto error; > } > @@ -342,7 +342,7 @@ void shutdown_pcifront(struct pcifront_dev *dev) > state = xenbus_read_integer(path); > while (err == NULL && state < XenbusStateClosing) > err = xenbus_wait_for_state_change(path, &state, &dev->events); > - if (err) free(err); > + free(err); > > if ((err = xenbus_switch_state(XBT_NIL, nodename, XenbusStateClosed)) != NULL) { > printk("shutdown_pcifront: error changing state to %d: %s\n", > @@ -365,16 +365,16 @@ void shutdown_pcifront(struct pcifront_dev *dev) > err = xenbus_wait_for_state_change(path, &state, &dev->events); > > close_pcifront: > - if (err) free(err); > + free(err); > err2 = xenbus_unwatch_path_token(XBT_NIL, path, path); > - if (err2) free(err2); > + free(err2); > > snprintf(nodename, sizeof(nodename), "%s/info-ref", dev->nodename); > err2 = xenbus_rm(XBT_NIL, nodename); > - if (err2) free(err2); > + free(err2); > snprintf(nodename, sizeof(nodename), "%s/event-channel", dev->nodename); > err2 = xenbus_rm(XBT_NIL, nodename); > - if (err2) free(err2); > + free(err2); > > if (!err) > free_pcifront(dev); > diff --git a/tools/blktap/drivers/blktapctrl.c b/tools/blktap/drivers/blktapctrl.c > index 0a8b880..0022264 100644 > --- a/tools/blktap/drivers/blktapctrl.c > +++ b/tools/blktap/drivers/blktapctrl.c > @@ -642,11 +642,9 @@ static int connect_tapdisk(blkif_t *blkif, int minor) > ret = 0; > > fail: > - if (rdctldev) > - free(rdctldev); > + free(rdctldev); > > - if (wrctldev) > - free(wrctldev); > + free(wrctldev); > > return ret; > } > diff --git a/tools/blktap/drivers/tapaio.c b/tools/blktap/drivers/tapaio.c > index 140c44a..ae26577 100644 > --- a/tools/blktap/drivers/tapaio.c > +++ b/tools/blktap/drivers/tapaio.c > @@ -244,18 +244,12 @@ fail: > > void tap_aio_free(tap_aio_context_t *ctx) > { > - if (ctx->sector_lock) > - free(ctx->sector_lock); > - if (ctx->iocb_list) > - free(ctx->iocb_list); > - if (ctx->pending_aio) > - free(ctx->pending_aio); > - if (ctx->aio_events) > - free(ctx->aio_events); > - if (ctx->iocb_free) > - free(ctx->iocb_free); > - if (ctx->iocb_queue) > - free(ctx->iocb_queue); > + free(ctx->sector_lock); > + free(ctx->iocb_list); > + free(ctx->pending_aio); > + free(ctx->aio_events); > + free(ctx->iocb_free); > + free(ctx->iocb_queue); > } > > /*TODO: Fix sector span!*/ > diff --git a/tools/blktap/drivers/tapdisk.c b/tools/blktap/drivers/tapdisk.c > index 19cd777..a6e8a7c 100644 > --- a/tools/blktap/drivers/tapdisk.c > +++ b/tools/blktap/drivers/tapdisk.c > @@ -82,10 +82,8 @@ static void daemonize(void) > > static void free_driver(struct disk_driver *d) > { > - if (d->name) > - free(d->name); > - if (d->private) > - free(d->private); > + free(d->name); > + free(d->private); > free(d); > } > > @@ -354,8 +352,7 @@ static int open_disk(struct td_state *s, > > fail: > DPRINTF("failed opening disk\n"); > - if (id.name) > - free(id.name); > + free(id.name); > d = s->disks; > while (d) { > struct disk_driver *tmp = d->next; > diff --git a/tools/blktap/lib/xenbus.c b/tools/blktap/lib/xenbus.c > index 948eb02..9edbf5a 100644 > --- a/tools/blktap/lib/xenbus.c > +++ b/tools/blktap/lib/xenbus.c > @@ -160,10 +160,8 @@ static int backend_remove(struct xs_handle *h, struct backend_info *be) > DPRINTF("Freeing blkif dev [%d]\n",be->blkif->devnum); > free_blkif(be->blkif); > } > - if (be->frontpath) > - free(be->frontpath); > - if (be->backpath) > - free(be->backpath); > + free(be->frontpath); > + free(be->backpath); > free(be); > return 0; > } > @@ -435,8 +433,7 @@ fail: > backend_remove(h, be); > } > close: > - if (path) > - free(path); > + free(path); > return; > } > > @@ -501,8 +498,7 @@ static void ueblktap_probe(struct xs_handle *h, struct xenbus_watch *w, > if ( (be != NULL) && (be->blkif != NULL) ) > backend_remove(h, be); > else goto free_be; > - if (bepath) > - free(bepath); > + free(bepath); > return; > } > > @@ -522,12 +518,9 @@ static void ueblktap_probe(struct xs_handle *h, struct xenbus_watch *w, > return; > > free_be: > - if (frontend) > - free(frontend); > - if (bepath) > - free(bepath); > - if (be) > - free(be); > + free(frontend); > + free(bepath); > + free(be); > } > > /** > diff --git a/tools/blktap/lib/xs_api.c b/tools/blktap/lib/xs_api.c > index 4648432..1624cb9 100644 > --- a/tools/blktap/lib/xs_api.c > +++ b/tools/blktap/lib/xs_api.c > @@ -204,8 +204,7 @@ char *get_dom_domid(struct xs_handle *h) > } > done: > xs_transaction_end(h, xth, 0); > - if (e) > - free(e); > + free(e); > return domid; > } > > diff --git a/tools/blktap2/control/tap-ctl-list.c b/tools/blktap2/control/tap-ctl-list.c > index c91f6f4..170735e 100644 > --- a/tools/blktap2/control/tap-ctl-list.c > +++ b/tools/blktap2/control/tap-ctl-list.c > @@ -219,8 +219,7 @@ out: > return err ? : n_minors; > > fail: > - if (minorv) > - free(minorv); > + free(minorv); > > goto out; > } > @@ -302,8 +301,7 @@ out: > return err ? : n_taps; > > fail: > - if (tapv) > - free(tapv); > + free(tapv); > > goto out; > } > diff --git a/tools/blktap2/drivers/block-log.c b/tools/blktap2/drivers/block-log.c > index 5330cdc..21c1946 100644 > --- a/tools/blktap2/drivers/block-log.c > +++ b/tools/blktap2/drivers/block-log.c > @@ -110,8 +110,7 @@ static int writelog_create(struct tdlog_state *s) > > static int writelog_free(struct tdlog_state *s) > { > - if (s->writelog) > - free(s->writelog); > + free(s->writelog); > > return 0; > } > diff --git a/tools/flask/utils/loadpolicy.c b/tools/flask/utils/loadpolicy.c > index f347b97..2edcf97 100644 > --- a/tools/flask/utils/loadpolicy.c > +++ b/tools/flask/utils/loadpolicy.c > @@ -108,8 +108,7 @@ int main (int argCnt, const char *args[]) > } > > done: > - if ( polMemCp ) > - free(polMemCp); > + free(polMemCp); > if ( polMem ) > { > ret = munmap(polMem, info.st_size); > diff --git a/tools/libxc/xc_compression.c b/tools/libxc/xc_compression.c > index 89f1be7..8f0b89d 100644 > --- a/tools/libxc/xc_compression.c > +++ b/tools/libxc/xc_compression.c > @@ -457,16 +457,11 @@ void xc_compression_free_context(xc_interface *xch, comp_ctx *ctx) > { > if (!ctx) return; > > - if (ctx->inputbuf) > - free(ctx->inputbuf); > - if (ctx->sendbuf_pfns) > - free(ctx->sendbuf_pfns); > - if (ctx->cache_base) > - free(ctx->cache_base); > - if (ctx->pfn2cache) > - free(ctx->pfn2cache); > - if (ctx->cache) > - free(ctx->cache); > + free(ctx->inputbuf); > + free(ctx->sendbuf_pfns); > + free(ctx->cache_base); > + free(ctx->pfn2cache); > + free(ctx->cache); > free(ctx); > } > > diff --git a/tools/libxc/xc_core_x86.c b/tools/libxc/xc_core_x86.c > index a9681f7..e328dcf 100644 > --- a/tools/libxc/xc_core_x86.c > +++ b/tools/libxc/xc_core_x86.c > @@ -180,11 +180,9 @@ out: > if ( live_p2m_frame_list ) > munmap(live_p2m_frame_list, P2M_FLL_ENTRIES * PAGE_SIZE); > > - if ( p2m_frame_list_list ) > - free(p2m_frame_list_list); > + free(p2m_frame_list_list); > > - if ( p2m_frame_list ) > - free(p2m_frame_list); > + free(p2m_frame_list); > > errno = err; > return ret; > diff --git a/tools/libxc/xc_domain_save.c b/tools/libxc/xc_domain_save.c > index fbc15e9..673d1e9 100644 > --- a/tools/libxc/xc_domain_save.c > +++ b/tools/libxc/xc_domain_save.c > @@ -773,11 +773,9 @@ static xen_pfn_t *map_and_save_p2m_table(xc_interface *xch, > if ( live_p2m_frame_list ) > munmap(live_p2m_frame_list, P2M_FLL_ENTRIES * PAGE_SIZE); > > - if ( p2m_frame_list_list ) > - free(p2m_frame_list_list); > + free(p2m_frame_list_list); > > - if ( p2m_frame_list ) > - free(p2m_frame_list); > + free(p2m_frame_list); > > return success ? p2m : NULL; > } > diff --git a/tools/libxc/xc_gnttab.c b/tools/libxc/xc_gnttab.c > index a8cc381..79dab40 100644 > --- a/tools/libxc/xc_gnttab.c > +++ b/tools/libxc/xc_gnttab.c > @@ -124,8 +124,7 @@ static void *_gnttab_map_table(xc_interface *xch, int domid, int *gnt_num) > err: > if ( frame_list ) > xc_hypercall_buffer_free(xch, frame_list); > - if ( pfn_list ) > - free(pfn_list); > + free(pfn_list); > > return gnt; > } > diff --git a/tools/libxc/xc_offline_page.c b/tools/libxc/xc_offline_page.c > index 8195efb..0b4cdf9 100644 > --- a/tools/libxc/xc_offline_page.c > +++ b/tools/libxc/xc_offline_page.c > @@ -626,14 +626,11 @@ failed: > } > } > > - if (mmu) > - free(mmu); > + free(mmu); > > - if (old_ptes.entries) > - free(old_ptes.entries); > + free(old_ptes.entries); > > - if (backup) > - free(backup); > + free(backup); > > if (gnttab_v1) > munmap(gnttab_v1, gnt_num / (PAGE_SIZE/sizeof(grant_entry_v1_t))); > diff --git a/tools/libxl/libxl_event.c b/tools/libxl/libxl_event.c > index fd2f280..73b7e63 100644 > --- a/tools/libxl/libxl_event.c > +++ b/tools/libxl/libxl_event.c > @@ -578,8 +578,7 @@ int libxl__ev_xswatch_register(libxl__gc *gc, libxl__ev_xswatch *w, > out_rc: > if (use) > LIBXL_SLIST_INSERT_HEAD(&CTX->watch_freeslots, use, empty); > - if (path_copy) > - free(path_copy); > + free(path_copy); > CTX_UNLOCK; > return rc; > } > diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c > index f681f3a..7e825ee 100644 > --- a/tools/libxl/libxl_qmp.c > +++ b/tools/libxl/libxl_qmp.c > @@ -406,12 +406,10 @@ static void qmp_close(libxl__qmp_handler *qmp) > > close(qmp->qmp_fd); > LIBXL_STAILQ_FOREACH(pp, &qmp->callback_list, next) { > - if (tmp) > - free(tmp); > + free(tmp); > tmp = pp; > } > - if (tmp) > - free(tmp); > + free(tmp); > } > > static int qmp_next(libxl__gc *gc, libxl__qmp_handler *qmp) > diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c > index 8e567a8..664a730 100644 > --- a/tools/libxl/libxl_utils.c > +++ b/tools/libxl/libxl_utils.c > @@ -355,7 +355,7 @@ int libxl_read_file_contents(libxl_ctx *ctx, const char *filename, > e = errno; > assert(e != ENOENT); > if (f) fclose(f); > - if (data) free(data); > + free(data); > return e; > } > > diff --git a/tools/memshr/bidir-hash.c b/tools/memshr/bidir-hash.c > index bed8179..3d34637 100644 > --- a/tools/memshr/bidir-hash.c > +++ b/tools/memshr/bidir-hash.c > @@ -208,8 +208,8 @@ static void free_buckets(struct __hash *h, > struct bucket *buckets, > struct bucket_lock *bucket_locks) > { > - if(buckets) free(buckets); > - if(bucket_locks) free(bucket_locks); > + free(buckets); > + free(bucket_locks); > } > > static int max_entries(struct __hash *h) > diff --git a/tools/misc/gtraceview.c b/tools/misc/gtraceview.c > index d8b4589..cf9287c 100644 > --- a/tools/misc/gtraceview.c > +++ b/tools/misc/gtraceview.c > @@ -959,8 +959,7 @@ int time_mode_rebuild(uint64_t start_time, uint64_t time_scale) > state = malloc(sizeof(struct state) * number); > if (!state) > return 1; > - if (this->state) > - free(this->state); > + free(this->state); > this->state = state; > this->width = 9; > this->row = 0; > diff --git a/tools/tests/xen-access/xen-access.c b/tools/tests/xen-access/xen-access.c > index 49195a8..b00c05a 100644 > --- a/tools/tests/xen-access/xen-access.c > +++ b/tools/tests/xen-access/xen-access.c > @@ -219,10 +219,8 @@ int xenaccess_teardown(xc_interface *xch, xenaccess_t *xenaccess) > } > xenaccess->xc_handle = NULL; > > - if ( xenaccess->platform_info ) > - free(xenaccess->platform_info); > - if ( xenaccess->domain_info ) > - free(xenaccess->domain_info); > + free(xenaccess->platform_info); > + free(xenaccess->domain_info); > free(xenaccess); > > return 0; > diff --git a/tools/xenbackendd/xenbackendd.c b/tools/xenbackendd/xenbackendd.c > index 5381a2a..f1eb1f5 100644 > --- a/tools/xenbackendd/xenbackendd.c > +++ b/tools/xenbackendd/xenbackendd.c > @@ -291,8 +291,7 @@ main(int argc, char * const argv[]) > > switch(type) { > case DEVTYPE_VIF: > - if (s) > - free(s); > + free(s); > snprintf(buf, sizeof(buf), "%s/script", > vec[XS_WATCH_PATH]); > s = xs_read(xs, XBT_NULL, buf, 0); > @@ -314,8 +313,7 @@ main(int argc, char * const argv[]) > } > > next2: > - if (s) > - free(s); > + free(s); > free(sstate); > > next1: > -- > 1.7.10.4 >-- Samuel <y> update-menus: relocation error: update-menus: symbol _ZNSt9basic_iosIcSt11char_traitsIcEE4initEPSt15basic_streambufIcS1_E, version GLIBCPP_3.2 not defined in file libstdc++.so.5 with link time reference <y> quoi que ça peut bien vouloir dire ? <D> N a eu la meme merde <y> c ça que ça veut dire ? wow, c''est bien crypté :) -+- #ens-mim s''entraide -+-
Ian Campbell
2013-Oct-03 13:39 UTC
Re: [PATCH v2] libxl: only put poller if already gotten in libxl_event_wait
On Sun, 2013-09-29 at 18:24 +1300, Matthew Daley wrote:> Coverity-ID: 1055292 > Signed-off-by: Matthew Daley <mattjd@gmail.com>Acked + applied, thanks.
Ian Campbell
2013-Oct-03 13:39 UTC
Re: [PATCH] libxc: only munmap when something has actually been mapped in change_pte
On Sun, 2013-09-29 at 14:35 +1300, Matthew Daley wrote:> Coverity-ID: 1055269 > Signed-off-by: Matthew Daley <mattjd@gmail.com> > --- > How''s this? It''s hard to create a very meaningful commit title here, IMO...Right,it''ll do. Acked + applied, thanks.
Ian Campbell
2013-Oct-03 13:43 UTC
Re: [PATCH v2] remove unnecessary null pointer checks before frees
On Sun, 2013-09-29 at 16:26 +1300, Matthew Daley wrote:> Oops, spatch removed an #if 0''d hunk from gtraceview.c. Here''s a fixed version: > > -- 8< -- > > Patch generated by the following semantic patch > (http://coccinelle.lip6.fr/): > > @@ > expression *P; > @@ > > - if(P) free(P); > + free(P); > > ...and then by filtering through the following command: > > filterdiff -p1 -x ''stubdom/*'' -x ''tools/firmware/*'' -x ''tools/qemu-*'' > > Signed-off-by: Matthew Daley <mattjd@gmail.com> > ---[...]> tools/blktap/drivers/blktapctrl.c | 6 ++---- > tools/blktap/drivers/tapaio.c | 18 ++++++---------- > tools/blktap/drivers/tapdisk.c | 9 +++----- > tools/blktap/lib/xenbus.c | 21 +++++++------------ > tools/blktap/lib/xs_api.c | 3 +-- > tools/blktap2/control/tap-ctl-list.c | 6 ++---- > tools/blktap2/drivers/block-log.c | 3 +--I''m slightly inclined to not touch these unmaintained bits, especially blktap1 from a "if it aint'' broke" type mentality and a reluctance to touch it even for such an obviously correct change. What do others think? The rest seems good. Ian.
Ian Campbell
2013-Oct-03 13:45 UTC
Re: [PATCH 00/28] Fixes for various minor Coverity issues, volume 2
On Wed, 2013-09-18 at 15:37 +1200, Matthew Daley wrote:> Tested by building and creating a few PV and HVM domains. There shouldn''t be > anything too complicated or controversial here. > > Matthew Daley (28): > gnttab: remove unused shared header lookupI have a feeling the hyp guys may have missed this one amount all the tools ones. By my reckoning all these have been applied except for:> libxl: only free cpupoolinfo if necessary in libxl_list_cpupool > libxl: only free cputopology if it was allocated in libxl__get_numa_candidate > libxl: only free cputopology if it was allocated in libxl_{cpu,node}map_to_{node,cpu}map > libxl: gettimeofday doesn''t return an errno on failureI think they are all waiting for an update version, is that right? Ian.
Ian Campbell
2013-Oct-03 13:47 UTC
Re: [PATCH 00/28] Fixes for various minor Coverity issues, volume 2
On Thu, 2013-10-03 at 14:45 +0100, Ian Campbell wrote:> On Wed, 2013-09-18 at 15:37 +1200, Matthew Daley wrote: > > Tested by building and creating a few PV and HVM domains. There shouldn''t be > > anything too complicated or controversial here. > > > > Matthew Daley (28): > > gnttab: remove unused shared header lookup > > I have a feeling the hyp guys may have missed this one amount all the > tools ones. > > By my reckoning all these have been applied except for: > > > libxl: only free cpupoolinfo if necessary in libxl_list_cpupool > > libxl: only free cputopology if it was allocated in libxl__get_numa_candidate > > libxl: only free cputopology if it was allocated in libxl_{cpu,node}map_to_{node,cpu}map > > libxl: gettimeofday doesn''t return an errno on failure > > I think they are all waiting for an update version, is that right?I''ve just spotted the resend of "libxl: only free cputopology if it was allocated in libxl_{cpu,node}map_to_{node,cpu}map". Looking at it now... Ian.
Ian Campbell
2013-Oct-03 14:03 UTC
Re: [PATCH v2] libxl: correctly handle libxl_get_cpu_topology failure in libxl_{cpu, node}map_to_{node, cpu}map
On Mon, 2013-09-30 at 12:29 +0200, Dario Faggioli wrote:> On dom, 2013-09-29 at 18:47 +1300, Matthew Daley wrote: > > Initialize nr_cpus to 0 so that if it is unchanged by a failing > > libxl_get_cpu_topology, libxl_cputopology_list_free still works OK > > afterward. > > > > Coverity-ID: 1055294 > > Coverity-ID: 1055295 > > Signed-off-by: Matthew Daley <mattjd@gmail.com> > > > Acked-by: Dario Faggioli <dario.faggioli@citrix.com>Applied, thanks.
Matthew Daley
2013-Oct-03 23:29 UTC
Re: [PATCH 00/28] Fixes for various minor Coverity issues, volume 2
On Fri, Oct 4, 2013 at 2:45 AM, Ian Campbell <Ian.Campbell@citrix.com> wrote:> On Wed, 2013-09-18 at 15:37 +1200, Matthew Daley wrote: >> Tested by building and creating a few PV and HVM domains. There shouldn''t be >> anything too complicated or controversial here. >> >> Matthew Daley (28): >> gnttab: remove unused shared header lookup > > I have a feeling the hyp guys may have missed this one amount all the > tools ones.Nope, it''s commited with ID 5ae3285f.> > By my reckoning all these have been applied except for: > >> libxl: only free cpupoolinfo if necessary in libxl_list_cpupoolPatch has been dropped, there''s no need for it>> libxl: only free cputopology if it was allocated in libxl__get_numa_candidateDitto>> libxl: only free cputopology if it was allocated in libxl_{cpu,node}map_to_{node,cpu}mapYou''ve just applied v2>> libxl: gettimeofday doesn''t return an errno on failureStill thinking about how to solve this and will send v2 when I get some free time. - Matthew> > I think they are all waiting for an update version, is that right? > > Ian. >
Ian Campbell
2013-Oct-04 08:00 UTC
Re: [PATCH 00/28] Fixes for various minor Coverity issues, volume 2
On Fri, 2013-10-04 at 12:29 +1300, Matthew Daley wrote:> >> gnttab: remove unused shared header lookup > Nope, it''s commited with ID 5ae3285f. > >> libxl: only free cpupoolinfo if necessary in libxl_list_cpupool > Patch has been dropped, there''s no need for it > >> libxl: only free cputopology if it was allocated in libxl__get_numa_candidate > Ditto > >> libxl: only free cputopology if it was allocated in libxl_{cpu,node}map_to_{node,cpu}map > You''ve just applied v2Thanks, removed all these from my queue.> >> libxl: gettimeofday doesn''t return an errno on failure > > Still thinking about how to solve this and will send v2 when I get > some free time.OK, thanks. Ian.
George Dunlap
2013-Oct-07 10:15 UTC
Re: [PATCH v2] remove unnecessary null pointer checks before frees
On Thu, Oct 3, 2013 at 2:43 PM, Ian Campbell <Ian.Campbell@citrix.com> wrote:> On Sun, 2013-09-29 at 16:26 +1300, Matthew Daley wrote: >> Oops, spatch removed an #if 0''d hunk from gtraceview.c. Here''s a fixed version: >> >> -- 8< -- >> >> Patch generated by the following semantic patch >> (http://coccinelle.lip6.fr/): >> >> @@ >> expression *P; >> @@ >> >> - if(P) free(P); >> + free(P); >> >> ...and then by filtering through the following command: >> >> filterdiff -p1 -x ''stubdom/*'' -x ''tools/firmware/*'' -x ''tools/qemu-*'' >> >> Signed-off-by: Matthew Daley <mattjd@gmail.com> >> --- > [...] >> tools/blktap/drivers/blktapctrl.c | 6 ++---- >> tools/blktap/drivers/tapaio.c | 18 ++++++---------- >> tools/blktap/drivers/tapdisk.c | 9 +++----- >> tools/blktap/lib/xenbus.c | 21 +++++++------------ >> tools/blktap/lib/xs_api.c | 3 +-- >> tools/blktap2/control/tap-ctl-list.c | 6 ++---- >> tools/blktap2/drivers/block-log.c | 3 +-- > > I''m slightly inclined to not touch these unmaintained bits, especially > blktap1 from a "if it aint'' broke" type mentality and a reluctance to > touch it even for such an obviously correct change. > > What do others think?That certainly sounds reasonable. -George
Ian Jackson
2013-Oct-11 11:12 UTC
[PATCH] libxl: make libxl__poller_put tolerate p==NULL libxl_event_wait
Ian Campbell writes ("Re: [PATCH v2] libxl: only put poller if already
gotten in libxl_event_wait"):> On Sun, 2013-09-29 at 18:24 +1300, Matthew Daley wrote:
> > Coverity-ID: 1055292
> > Signed-off-by: Matthew Daley <mattjd@gmail.com>
>
> Acked + applied, thanks.
commit 9134bb273db4d55bec247d751b1de6e7876a7fe7
Author: Ian Jackson <ian.jackson@eu.citrix.com>
Date: Fri Oct 11 12:10:45 2013 +0100
libxl: make libxl__poller_put tolerate p==NULL
This is less fragile, and more in keeping with the usual style of
initialising everything to 0 and freeing things unconditionally.
Correspondingly, remove the tests at the call sites.
Apropos of c1f3f174. No overall functional change.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
diff --git a/tools/libxl/libxl_event.c b/tools/libxl/libxl_event.c
index 6f033dd..a5c52bc 100644
--- a/tools/libxl/libxl_event.c
+++ b/tools/libxl/libxl_event.c
@@ -1347,6 +1347,7 @@ libxl__poller *libxl__poller_get(libxl_ctx *ctx)
void libxl__poller_put(libxl_ctx *ctx, libxl__poller *p)
{
+ if (!p) return;
LIBXL_LIST_INSERT_HEAD(&ctx->pollers_idle, p, entry);
}
@@ -1476,8 +1477,7 @@ int libxl_event_wait(libxl_ctx *ctx, libxl_event
**event_r,
}
out:
- if (poller)
- libxl__poller_put(ctx, poller);
+ libxl__poller_put(ctx, poller);
CTX_UNLOCK;
EGC_FREE;
@@ -1540,7 +1540,7 @@ void libxl__ao__destroy(libxl_ctx *ctx, libxl__ao *ao)
AO_GC;
if (!ao) return;
LOG(DEBUG,"ao %p: destroy",ao);
- if (ao->poller) libxl__poller_put(ctx, ao->poller);
+ libxl__poller_put(ctx, ao->poller);
ao->magic = LIBXL__AO_MAGIC_DESTROYED;
libxl__free_all(&ao->gc);
free(ao);
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 4e15055..165dc00 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -829,7 +829,7 @@ _hidden void libxl__poller_dispose(libxl__poller *p);
* away again afterwards. _get can fail, returning NULL.
* ctx must be locked. */
_hidden libxl__poller *libxl__poller_get(libxl_ctx *ctx);
-_hidden void libxl__poller_put(libxl_ctx *ctx, libxl__poller *p);
+_hidden void libxl__poller_put(libxl_ctx*, libxl__poller *p /* may be NULL */);
/* Notifies whoever is polling using p that they should wake up.
* ctx must be locked. */
Ian Campbell
2013-Oct-11 11:47 UTC
Re: [PATCH] libxl: make libxl__poller_put tolerate p==NULL libxl_event_wait
On Fri, 2013-10-11 at 12:12 +0100, Ian Jackson wrote:> Ian Campbell writes ("Re: [PATCH v2] libxl: only put poller if already gotten in libxl_event_wait"): > > On Sun, 2013-09-29 at 18:24 +1300, Matthew Daley wrote: > > > Coverity-ID: 1055292 > > > Signed-off-by: Matthew Daley <mattjd@gmail.com> > > > > Acked + applied, thanks. > > commit 9134bb273db4d55bec247d751b1de6e7876a7fe7 > Author: Ian Jackson <ian.jackson@eu.citrix.com> > Date: Fri Oct 11 12:10:45 2013 +0100 > > libxl: make libxl__poller_put tolerate p==NULL > > This is less fragile, and more in keeping with the usual style of > initialising everything to 0 and freeing things unconditionally. > > Correspondingly, remove the tests at the call sites. > > Apropos of c1f3f174. No overall functional change. > > Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>Yes, good idea. Acked-by: Ian Campbell <ian.campbell@citrix.com>
Ian Jackson
2013-Oct-11 14:51 UTC
Re: [PATCH] libxl: make libxl__poller_put tolerate p==NULL libxl_event_wait
Ian Campbell writes ("Re: [PATCH] libxl: make libxl__poller_put tolerate
p==NULL libxl_event_wait"):> On Fri, 2013-10-11 at 12:12 +0100, Ian Jackson wrote:
> > commit 9134bb273db4d55bec247d751b1de6e7876a7fe7
> > Author: Ian Jackson <ian.jackson@eu.citrix.com>
> > Date: Fri Oct 11 12:10:45 2013 +0100
> >
> > libxl: make libxl__poller_put tolerate p==NULL
> >
> > This is less fragile, and more in keeping with the usual style of
> > initialising everything to 0 and freeing things unconditionally.
> >
> > Correspondingly, remove the tests at the call sites.
> >
> > Apropos of c1f3f174. No overall functional change.
> >
> > Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
>
> Yes, good idea.
>
> Acked-by: Ian Campbell <ian.campbell@citrix.com>
Pushed, thanks.
Ian.
Matthew Daley
2013-Oct-12 22:38 UTC
Re: [PATCH] libxl: make libxl__poller_put tolerate p==NULL libxl_event_wait
On Sat, Oct 12, 2013 at 12:12 AM, Ian Jackson <Ian.Jackson@eu.citrix.com> wrote:> commit 9134bb273db4d55bec247d751b1de6e7876a7fe7 > Author: Ian Jackson <ian.jackson@eu.citrix.com> > Date: Fri Oct 11 12:10:45 2013 +0100 > > libxl: make libxl__poller_put tolerate p==NULL > > This is less fragile, and more in keeping with the usual style of > initialising everything to 0 and freeing things unconditionally. > > Correspondingly, remove the tests at the call sites. > > Apropos of c1f3f174. No overall functional change. > > Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com> > > diff --git a/tools/libxl/libxl_event.c b/tools/libxl/libxl_event.c > index 6f033dd..a5c52bc 100644 > --- a/tools/libxl/libxl_event.c > +++ b/tools/libxl/libxl_event.c > @@ -1347,6 +1347,7 @@ libxl__poller *libxl__poller_get(libxl_ctx *ctx) > > void libxl__poller_put(libxl_ctx *ctx, libxl__poller *p) > { > + if (!p) return;So before I go and send more libxl patches, what''s the story with single-line if statements like this? Is it acceptable in libxl (only?) after all? - Matthew
Ian Jackson
2013-Oct-14 16:49 UTC
Re: [PATCH] libxl: make libxl__poller_put tolerate p==NULL libxl_event_wait
Matthew Daley writes ("Re: [PATCH] libxl: make libxl__poller_put tolerate
p==NULL libxl_event_wait"):> On Sat, Oct 12, 2013 at 12:12 AM, Ian Jackson
<Ian.Jackson@eu.citrix.com> wrote:
> > void libxl__poller_put(libxl_ctx *ctx, libxl__poller *p)
> > {
> > + if (!p) return;
>
> So before I go and send more libxl patches, what''s the story with
> single-line if statements like this? Is it acceptable in libxl (only?)
> after all?
From tools/libxl/CODING_STYLE:
4. Statements
Don''t put multiple statements on a single line.
Don''t put multiple assignments on a single line either.
Error code paths with an if statement and a goto or a return on the same
line are allowed. Examples:
if (rc) goto out;
Whether you think this is an error path is a matter of taste, I think.
Ian.
Matthew Daley
2013-Oct-15 05:18 UTC
[PATCH v3] remove unnecessary null pointer checks before frees
Patch generated by the following semantic patch
(http://coccinelle.lip6.fr/):
@@
expression *P;
@@
- if(P) free(P);
+ free(P);
...and then by filtering through the following command:
filterdiff -p1 -x ''stubdom/*'' -x
''tools/firmware/*'' -x ''tools/qemu-*'' -x
''tools/blktap*''
Signed-off-by: Matthew Daley <mattjd@gmail.com>
---
extras/mini-os/blkfront.c | 14 ++++++-------
extras/mini-os/console/xenbus.c | 8 ++++----
extras/mini-os/fbfront.c | 38 +++++++++++++++++------------------
extras/mini-os/kernel.c | 4 ++--
extras/mini-os/lib/sys.c | 3 +--
extras/mini-os/netfront.c | 18 ++++++++---------
extras/mini-os/pcifront.c | 14 ++++++-------
tools/flask/utils/loadpolicy.c | 3 +--
tools/libxc/xc_compression.c | 15 +++++---------
tools/libxc/xc_core_x86.c | 6 ++----
tools/libxc/xc_domain_save.c | 6 ++----
tools/libxc/xc_gnttab.c | 3 +--
tools/libxc/xc_offline_page.c | 9 +++------
tools/libxl/libxl_event.c | 3 +--
tools/libxl/libxl_qmp.c | 6 ++----
tools/libxl/libxl_utils.c | 2 +-
tools/memshr/bidir-hash.c | 4 ++--
tools/misc/gtraceview.c | 3 +--
tools/tests/xen-access/xen-access.c | 6 ++----
tools/xenbackendd/xenbackendd.c | 6 ++----
20 files changed, 74 insertions(+), 97 deletions(-)
diff --git a/extras/mini-os/blkfront.c b/extras/mini-os/blkfront.c
index aead6bd..62a32c5 100644
--- a/extras/mini-os/blkfront.c
+++ b/extras/mini-os/blkfront.c
@@ -160,7 +160,7 @@ again:
err = xenbus_transaction_end(xbt, 0, &retry);
- if (err) free(err);
+ free(err);
if (retry) {
goto again;
printk("completing transaction\n");
@@ -271,7 +271,7 @@ void shutdown_blkfront(struct blkfront_dev *dev)
state = xenbus_read_integer(path);
while (err == NULL && state < XenbusStateClosing)
err = xenbus_wait_for_state_change(path, &state,
&dev->events);
- if (err) free(err);
+ free(err);
if ((err = xenbus_switch_state(XBT_NIL, nodename, XenbusStateClosed)) !=
NULL) {
printk("shutdown_blkfront: error changing state to %d: %s\n",
@@ -281,7 +281,7 @@ void shutdown_blkfront(struct blkfront_dev *dev)
state = xenbus_read_integer(path);
while (state < XenbusStateClosed) {
err = xenbus_wait_for_state_change(path, &state,
&dev->events);
- if (err) free(err);
+ free(err);
}
if ((err = xenbus_switch_state(XBT_NIL, nodename, XenbusStateInitialising))
!= NULL) {
@@ -294,16 +294,16 @@ void shutdown_blkfront(struct blkfront_dev *dev)
err = xenbus_wait_for_state_change(path, &state,
&dev->events);
close:
- if (err) free(err);
+ free(err);
err2 = xenbus_unwatch_path_token(XBT_NIL, path, path);
- if (err2) free(err2);
+ free(err2);
snprintf(nodename, sizeof(nodename), "%s/ring-ref",
dev->nodename);
err2 = xenbus_rm(XBT_NIL, nodename);
- if (err2) free(err2);
+ free(err2);
snprintf(nodename, sizeof(nodename), "%s/event-channel",
dev->nodename);
err2 = xenbus_rm(XBT_NIL, nodename);
- if (err2) free(err2);
+ free(err2);
if (!err)
free_blkfront(dev);
diff --git a/extras/mini-os/console/xenbus.c b/extras/mini-os/console/xenbus.c
index b317114..1c9a590 100644
--- a/extras/mini-os/console/xenbus.c
+++ b/extras/mini-os/console/xenbus.c
@@ -32,7 +32,7 @@ void free_consfront(struct consfront_dev *dev)
state = xenbus_read_integer(path);
while (err == NULL && state < XenbusStateClosing)
err = xenbus_wait_for_state_change(path, &state,
&dev->events);
- if (err) free(err);
+ free(err);
if ((err = xenbus_switch_state(XBT_NIL, nodename, XenbusStateClosed)) !=
NULL) {
printk("free_consfront: error changing state to %d: %s\n",
@@ -41,9 +41,9 @@ void free_consfront(struct consfront_dev *dev)
}
close:
- if (err) free(err);
+ free(err);
err = xenbus_unwatch_path_token(XBT_NIL, path, path);
- if (err) free(err);
+ free(err);
mask_evtchn(dev->evtchn);
unbind_evtchn(dev->evtchn);
@@ -134,7 +134,7 @@ again:
err = xenbus_transaction_end(xbt, 0, &retry);
- if (err) free(err);
+ free(err);
if (retry) {
goto again;
printk("completing transaction\n");
diff --git a/extras/mini-os/fbfront.c b/extras/mini-os/fbfront.c
index a276193..1e01513 100644
--- a/extras/mini-os/fbfront.c
+++ b/extras/mini-os/fbfront.c
@@ -131,7 +131,7 @@ again:
}
err = xenbus_transaction_end(xbt, 0, &retry);
- if (err) free(err);
+ free(err);
if (retry) {
goto again;
printk("completing transaction\n");
@@ -171,7 +171,7 @@ done:
err = xenbus_wait_for_state_change(path, &state,
&dev->events);
if (state != XenbusStateConnected) {
printk("backend not available, state=%d\n", state);
- if (err) free(err);
+ free(err);
err = xenbus_unwatch_path_token(XBT_NIL, path, path);
goto error;
}
@@ -256,7 +256,7 @@ void shutdown_kbdfront(struct kbdfront_dev *dev)
state = xenbus_read_integer(path);
while (err == NULL && state < XenbusStateClosing)
err = xenbus_wait_for_state_change(path, &state,
&dev->events);
- if (err) free(err);
+ free(err);
if ((err = xenbus_switch_state(XBT_NIL, nodename, XenbusStateClosed)) !=
NULL) {
printk("shutdown_kbdfront: error changing state to %d: %s\n",
@@ -266,7 +266,7 @@ void shutdown_kbdfront(struct kbdfront_dev *dev)
state = xenbus_read_integer(path);
while (state < XenbusStateClosed) {
err = xenbus_wait_for_state_change(path, &state,
&dev->events);
- if (err) free(err);
+ free(err);
}
if ((err = xenbus_switch_state(XBT_NIL, nodename, XenbusStateInitialising))
!= NULL) {
@@ -279,19 +279,19 @@ void shutdown_kbdfront(struct kbdfront_dev *dev)
err = xenbus_wait_for_state_change(path, &state, &dev->events);
close_kbdfront:
- if (err) free(err);
+ free(err);
err2 = xenbus_unwatch_path_token(XBT_NIL, path, path);
- if (err2) free(err2);
+ free(err2);
snprintf(nodename, sizeof(nodename), "%s/page-ref",
dev->nodename);
err2 = xenbus_rm(XBT_NIL, nodename);
- if (err2) free(err2);
+ free(err2);
snprintf(nodename, sizeof(nodename), "%s/event-channel",
dev->nodename);
err2 = xenbus_rm(XBT_NIL, nodename);
- if (err2) free(err2);
+ free(err2);
snprintf(nodename, sizeof(nodename), "%s/request-abs-pointer",
dev->nodename);
err2 = xenbus_rm(XBT_NIL, nodename);
- if (err2) free(err2);
+ free(err2);
if (!err)
free_kbdfront(dev);
@@ -498,7 +498,7 @@ again:
}
err = xenbus_transaction_end(xbt, 0, &retry);
- if (err) free(err);
+ free(err);
if (retry) {
goto again;
printk("completing transaction\n");
@@ -538,7 +538,7 @@ done:
err = xenbus_wait_for_state_change(path, &state,
&dev->events);
if (state != XenbusStateConnected) {
printk("backend not available, state=%d\n", state);
- if (err) free(err);
+ free(err);
err = xenbus_unwatch_path_token(XBT_NIL, path, path);
goto error;
}
@@ -654,7 +654,7 @@ void shutdown_fbfront(struct fbfront_dev *dev)
state = xenbus_read_integer(path);
while (err == NULL && state < XenbusStateClosing)
err = xenbus_wait_for_state_change(path, &state,
&dev->events);
- if (err) free(err);
+ free(err);
if ((err = xenbus_switch_state(XBT_NIL, nodename, XenbusStateClosed)) !=
NULL) {
printk("shutdown_fbfront: error changing state to %d: %s\n",
@@ -664,7 +664,7 @@ void shutdown_fbfront(struct fbfront_dev *dev)
state = xenbus_read_integer(path);
if (state < XenbusStateClosed) {
err = xenbus_wait_for_state_change(path, &state,
&dev->events);
- if (err) free(err);
+ free(err);
}
if ((err = xenbus_switch_state(XBT_NIL, nodename, XenbusStateInitialising))
!= NULL) {
@@ -677,22 +677,22 @@ void shutdown_fbfront(struct fbfront_dev *dev)
err = xenbus_wait_for_state_change(path, &state,
&dev->events);
close_fbfront:
- if (err) free(err);
+ free(err);
err2 = xenbus_unwatch_path_token(XBT_NIL, path, path);
- if (err2) free(err2);
+ free(err2);
snprintf(nodename, sizeof(nodename), "%s/page-ref",
dev->nodename);
err2 = xenbus_rm(XBT_NIL, nodename);
- if (err2) free(err2);
+ free(err2);
snprintf(nodename, sizeof(nodename), "%s/event-channel",
dev->nodename);
err2 = xenbus_rm(XBT_NIL, nodename);
- if (err2) free(err2);
+ free(err2);
snprintf(nodename, sizeof(nodename), "%s/protocol",
dev->nodename);
err2 = xenbus_rm(XBT_NIL, nodename);
- if (err2) free(err2);
+ free(err2);
snprintf(nodename, sizeof(nodename), "%s/feature-update",
dev->nodename);
err2 = xenbus_rm(XBT_NIL, nodename);
- if (err2) free(err2);
+ free(err2);
if (!err)
free_fbfront(dev);
diff --git a/extras/mini-os/kernel.c b/extras/mini-os/kernel.c
index 24fa25c..ea409f4 100644
--- a/extras/mini-os/kernel.c
+++ b/extras/mini-os/kernel.c
@@ -85,9 +85,9 @@ static void shutdown_thread(void *p)
xenbus_wait_for_watch(&events);
}
err = xenbus_unwatch_path_token(XBT_NIL, path, token);
- if (err) free(err);
+ free(err);
err = xenbus_write(XBT_NIL, path, "");
- if (err) free(err);
+ free(err);
printk("Shutting down (%s)\n", shutdown);
if (!strcmp(shutdown, "poweroff"))
diff --git a/extras/mini-os/lib/sys.c b/extras/mini-os/lib/sys.c
index cfbdc90..13e7e59 100644
--- a/extras/mini-os/lib/sys.c
+++ b/extras/mini-os/lib/sys.c
@@ -1156,8 +1156,7 @@ LWIP_STUB(int, getsockname, (int s, struct sockaddr *name,
socklen_t *namelen),
static char *syslog_ident;
void openlog(const char *ident, int option, int facility)
{
- if (syslog_ident)
- free(syslog_ident);
+ free(syslog_ident);
syslog_ident = strdup(ident);
}
diff --git a/extras/mini-os/netfront.c b/extras/mini-os/netfront.c
index 3a5be64..44c3995 100644
--- a/extras/mini-os/netfront.c
+++ b/extras/mini-os/netfront.c
@@ -412,7 +412,7 @@ again:
}
err = xenbus_transaction_end(xbt, 0, &retry);
- if (err) free(err);
+ free(err);
if (retry) {
goto again;
printk("completing transaction\n");
@@ -525,7 +525,7 @@ void shutdown_netfront(struct netfront_dev *dev)
state = xenbus_read_integer(path);
while (err == NULL && state < XenbusStateClosing)
err = xenbus_wait_for_state_change(path, &state,
&dev->events);
- if (err) free(err);
+ free(err);
if ((err = xenbus_switch_state(XBT_NIL, nodename, XenbusStateClosed)) !=
NULL) {
printk("shutdown_netfront: error changing state to %d: %s\n",
@@ -535,7 +535,7 @@ void shutdown_netfront(struct netfront_dev *dev)
state = xenbus_read_integer(path);
while (state < XenbusStateClosed) {
err = xenbus_wait_for_state_change(path, &state,
&dev->events);
- if (err) free(err);
+ free(err);
}
if ((err = xenbus_switch_state(XBT_NIL, nodename, XenbusStateInitialising))
!= NULL) {
@@ -548,22 +548,22 @@ void shutdown_netfront(struct netfront_dev *dev)
err = xenbus_wait_for_state_change(path, &state,
&dev->events);
close:
- if (err) free(err);
+ free(err);
err2 = xenbus_unwatch_path_token(XBT_NIL, path, path);
- if (err2) free(err2);
+ free(err2);
snprintf(nodename, sizeof(nodename), "%s/tx-ring-ref",
dev->nodename);
err2 = xenbus_rm(XBT_NIL, nodename);
- if (err2) free(err2);
+ free(err2);
snprintf(nodename, sizeof(nodename), "%s/rx-ring-ref",
dev->nodename);
err2 = xenbus_rm(XBT_NIL, nodename);
- if (err2) free(err2);
+ free(err2);
snprintf(nodename, sizeof(nodename), "%s/event-channel",
dev->nodename);
err2 = xenbus_rm(XBT_NIL, nodename);
- if (err2) free(err2);
+ free(err2);
snprintf(nodename, sizeof(nodename), "%s/request-rx-copy",
dev->nodename);
err2 = xenbus_rm(XBT_NIL, nodename);
- if (err2) free(err2);
+ free(err2);
if (!err)
free_netfront(dev);
diff --git a/extras/mini-os/pcifront.c b/extras/mini-os/pcifront.c
index 7838021..16a4b49 100644
--- a/extras/mini-os/pcifront.c
+++ b/extras/mini-os/pcifront.c
@@ -212,7 +212,7 @@ again:
}
err = xenbus_transaction_end(xbt, 0, &retry);
- if (err) free(err);
+ free(err);
if (retry) {
goto again;
printk("completing transaction\n");
@@ -251,7 +251,7 @@ done:
err = xenbus_wait_for_state_change(path, &state,
&dev->events);
if (state != XenbusStateConnected) {
printk("backend not avalable, state=%d\n", state);
- if (err) free(err);
+ free(err);
err = xenbus_unwatch_path_token(XBT_NIL, path, path);
goto error;
}
@@ -342,7 +342,7 @@ void shutdown_pcifront(struct pcifront_dev *dev)
state = xenbus_read_integer(path);
while (err == NULL && state < XenbusStateClosing)
err = xenbus_wait_for_state_change(path, &state,
&dev->events);
- if (err) free(err);
+ free(err);
if ((err = xenbus_switch_state(XBT_NIL, nodename, XenbusStateClosed)) !=
NULL) {
printk("shutdown_pcifront: error changing state to %d: %s\n",
@@ -365,16 +365,16 @@ void shutdown_pcifront(struct pcifront_dev *dev)
err = xenbus_wait_for_state_change(path, &state,
&dev->events);
close_pcifront:
- if (err) free(err);
+ free(err);
err2 = xenbus_unwatch_path_token(XBT_NIL, path, path);
- if (err2) free(err2);
+ free(err2);
snprintf(nodename, sizeof(nodename), "%s/info-ref",
dev->nodename);
err2 = xenbus_rm(XBT_NIL, nodename);
- if (err2) free(err2);
+ free(err2);
snprintf(nodename, sizeof(nodename), "%s/event-channel",
dev->nodename);
err2 = xenbus_rm(XBT_NIL, nodename);
- if (err2) free(err2);
+ free(err2);
if (!err)
free_pcifront(dev);
diff --git a/tools/flask/utils/loadpolicy.c b/tools/flask/utils/loadpolicy.c
index f347b97..2edcf97 100644
--- a/tools/flask/utils/loadpolicy.c
+++ b/tools/flask/utils/loadpolicy.c
@@ -108,8 +108,7 @@ int main (int argCnt, const char *args[])
}
done:
- if ( polMemCp )
- free(polMemCp);
+ free(polMemCp);
if ( polMem )
{
ret = munmap(polMem, info.st_size);
diff --git a/tools/libxc/xc_compression.c b/tools/libxc/xc_compression.c
index 89f1be7..8f0b89d 100644
--- a/tools/libxc/xc_compression.c
+++ b/tools/libxc/xc_compression.c
@@ -457,16 +457,11 @@ void xc_compression_free_context(xc_interface *xch,
comp_ctx *ctx)
{
if (!ctx) return;
- if (ctx->inputbuf)
- free(ctx->inputbuf);
- if (ctx->sendbuf_pfns)
- free(ctx->sendbuf_pfns);
- if (ctx->cache_base)
- free(ctx->cache_base);
- if (ctx->pfn2cache)
- free(ctx->pfn2cache);
- if (ctx->cache)
- free(ctx->cache);
+ free(ctx->inputbuf);
+ free(ctx->sendbuf_pfns);
+ free(ctx->cache_base);
+ free(ctx->pfn2cache);
+ free(ctx->cache);
free(ctx);
}
diff --git a/tools/libxc/xc_core_x86.c b/tools/libxc/xc_core_x86.c
index a9681f7..e328dcf 100644
--- a/tools/libxc/xc_core_x86.c
+++ b/tools/libxc/xc_core_x86.c
@@ -180,11 +180,9 @@ out:
if ( live_p2m_frame_list )
munmap(live_p2m_frame_list, P2M_FLL_ENTRIES * PAGE_SIZE);
- if ( p2m_frame_list_list )
- free(p2m_frame_list_list);
+ free(p2m_frame_list_list);
- if ( p2m_frame_list )
- free(p2m_frame_list);
+ free(p2m_frame_list);
errno = err;
return ret;
diff --git a/tools/libxc/xc_domain_save.c b/tools/libxc/xc_domain_save.c
index fbc15e9..673d1e9 100644
--- a/tools/libxc/xc_domain_save.c
+++ b/tools/libxc/xc_domain_save.c
@@ -773,11 +773,9 @@ static xen_pfn_t *map_and_save_p2m_table(xc_interface *xch,
if ( live_p2m_frame_list )
munmap(live_p2m_frame_list, P2M_FLL_ENTRIES * PAGE_SIZE);
- if ( p2m_frame_list_list )
- free(p2m_frame_list_list);
+ free(p2m_frame_list_list);
- if ( p2m_frame_list )
- free(p2m_frame_list);
+ free(p2m_frame_list);
return success ? p2m : NULL;
}
diff --git a/tools/libxc/xc_gnttab.c b/tools/libxc/xc_gnttab.c
index a8cc381..79dab40 100644
--- a/tools/libxc/xc_gnttab.c
+++ b/tools/libxc/xc_gnttab.c
@@ -124,8 +124,7 @@ static void *_gnttab_map_table(xc_interface *xch, int domid,
int *gnt_num)
err:
if ( frame_list )
xc_hypercall_buffer_free(xch, frame_list);
- if ( pfn_list )
- free(pfn_list);
+ free(pfn_list);
return gnt;
}
diff --git a/tools/libxc/xc_offline_page.c b/tools/libxc/xc_offline_page.c
index 8195efb..0b4cdf9 100644
--- a/tools/libxc/xc_offline_page.c
+++ b/tools/libxc/xc_offline_page.c
@@ -626,14 +626,11 @@ failed:
}
}
- if (mmu)
- free(mmu);
+ free(mmu);
- if (old_ptes.entries)
- free(old_ptes.entries);
+ free(old_ptes.entries);
- if (backup)
- free(backup);
+ free(backup);
if (gnttab_v1)
munmap(gnttab_v1, gnt_num / (PAGE_SIZE/sizeof(grant_entry_v1_t)));
diff --git a/tools/libxl/libxl_event.c b/tools/libxl/libxl_event.c
index a5c52bc..c5e9426 100644
--- a/tools/libxl/libxl_event.c
+++ b/tools/libxl/libxl_event.c
@@ -578,8 +578,7 @@ int libxl__ev_xswatch_register(libxl__gc *gc,
libxl__ev_xswatch *w,
out_rc:
if (use)
LIBXL_SLIST_INSERT_HEAD(&CTX->watch_freeslots, use, empty);
- if (path_copy)
- free(path_copy);
+ free(path_copy);
CTX_UNLOCK;
return rc;
}
diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
index f681f3a..7e825ee 100644
--- a/tools/libxl/libxl_qmp.c
+++ b/tools/libxl/libxl_qmp.c
@@ -406,12 +406,10 @@ static void qmp_close(libxl__qmp_handler *qmp)
close(qmp->qmp_fd);
LIBXL_STAILQ_FOREACH(pp, &qmp->callback_list, next) {
- if (tmp)
- free(tmp);
+ free(tmp);
tmp = pp;
}
- if (tmp)
- free(tmp);
+ free(tmp);
}
static int qmp_next(libxl__gc *gc, libxl__qmp_handler *qmp)
diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
index 1bcac7e..682f874 100644
--- a/tools/libxl/libxl_utils.c
+++ b/tools/libxl/libxl_utils.c
@@ -355,7 +355,7 @@ int libxl_read_file_contents(libxl_ctx *ctx, const char
*filename,
e = errno;
assert(e != ENOENT);
if (f) fclose(f);
- if (data) free(data);
+ free(data);
return e;
}
diff --git a/tools/memshr/bidir-hash.c b/tools/memshr/bidir-hash.c
index bed8179..3d34637 100644
--- a/tools/memshr/bidir-hash.c
+++ b/tools/memshr/bidir-hash.c
@@ -208,8 +208,8 @@ static void free_buckets(struct __hash *h,
struct bucket *buckets,
struct bucket_lock *bucket_locks)
{
- if(buckets) free(buckets);
- if(bucket_locks) free(bucket_locks);
+ free(buckets);
+ free(bucket_locks);
}
static int max_entries(struct __hash *h)
diff --git a/tools/misc/gtraceview.c b/tools/misc/gtraceview.c
index d8b4589..cf9287c 100644
--- a/tools/misc/gtraceview.c
+++ b/tools/misc/gtraceview.c
@@ -959,8 +959,7 @@ int time_mode_rebuild(uint64_t start_time, uint64_t
time_scale)
state = malloc(sizeof(struct state) * number);
if (!state)
return 1;
- if (this->state)
- free(this->state);
+ free(this->state);
this->state = state;
this->width = 9;
this->row = 0;
diff --git a/tools/tests/xen-access/xen-access.c
b/tools/tests/xen-access/xen-access.c
index 49195a8..b00c05a 100644
--- a/tools/tests/xen-access/xen-access.c
+++ b/tools/tests/xen-access/xen-access.c
@@ -219,10 +219,8 @@ int xenaccess_teardown(xc_interface *xch, xenaccess_t
*xenaccess)
}
xenaccess->xc_handle = NULL;
- if ( xenaccess->platform_info )
- free(xenaccess->platform_info);
- if ( xenaccess->domain_info )
- free(xenaccess->domain_info);
+ free(xenaccess->platform_info);
+ free(xenaccess->domain_info);
free(xenaccess);
return 0;
diff --git a/tools/xenbackendd/xenbackendd.c b/tools/xenbackendd/xenbackendd.c
index 5381a2a..f1eb1f5 100644
--- a/tools/xenbackendd/xenbackendd.c
+++ b/tools/xenbackendd/xenbackendd.c
@@ -291,8 +291,7 @@ main(int argc, char * const argv[])
switch(type) {
case DEVTYPE_VIF:
- if (s)
- free(s);
+ free(s);
snprintf(buf, sizeof(buf), "%s/script",
vec[XS_WATCH_PATH]);
s = xs_read(xs, XBT_NULL, buf, 0);
@@ -314,8 +313,7 @@ main(int argc, char * const argv[])
}
next2:
- if (s)
- free(s);
+ free(s);
free(sstate);
next1:
--
1.7.10.4
Ian Campbell
2013-Oct-31 22:03 UTC
Re: [PATCH v3] remove unnecessary null pointer checks before frees
On Tue, 2013-10-15 at 18:18 +1300, Matthew Daley wrote:> Patch generated by the following semantic patch > (http://coccinelle.lip6.fr/): > > @@ > expression *P; > @@ > > - if(P) free(P); > + free(P); > > ...and then by filtering through the following command: > > filterdiff -p1 -x ''stubdom/*'' -x ''tools/firmware/*'' -x ''tools/qemu-*'' -x ''tools/blktap*'' > > Signed-off-by: Matthew Daley <mattjd@gmail.com>No one seems to be objecting, so lets give it a go: Acked-by: Ian Campbell <ian.campbell@citrix.com> I also picked up Samuel''s ack from v2 since the mini-os bits are still valid. Pushed.> --- > extras/mini-os/blkfront.c | 14 ++++++------- > extras/mini-os/console/xenbus.c | 8 ++++---- > extras/mini-os/fbfront.c | 38 +++++++++++++++++------------------ > extras/mini-os/kernel.c | 4 ++-- > extras/mini-os/lib/sys.c | 3 +-- > extras/mini-os/netfront.c | 18 ++++++++--------- > extras/mini-os/pcifront.c | 14 ++++++------- > tools/flask/utils/loadpolicy.c | 3 +-- > tools/libxc/xc_compression.c | 15 +++++--------- > tools/libxc/xc_core_x86.c | 6 ++---- > tools/libxc/xc_domain_save.c | 6 ++---- > tools/libxc/xc_gnttab.c | 3 +-- > tools/libxc/xc_offline_page.c | 9 +++------ > tools/libxl/libxl_event.c | 3 +-- > tools/libxl/libxl_qmp.c | 6 ++---- > tools/libxl/libxl_utils.c | 2 +- > tools/memshr/bidir-hash.c | 4 ++-- > tools/misc/gtraceview.c | 3 +-- > tools/tests/xen-access/xen-access.c | 6 ++---- > tools/xenbackendd/xenbackendd.c | 6 ++---- > 20 files changed, 74 insertions(+), 97 deletions(-) > > diff --git a/extras/mini-os/blkfront.c b/extras/mini-os/blkfront.c > index aead6bd..62a32c5 100644 > --- a/extras/mini-os/blkfront.c > +++ b/extras/mini-os/blkfront.c > @@ -160,7 +160,7 @@ again: > > > err = xenbus_transaction_end(xbt, 0, &retry); > - if (err) free(err); > + free(err); > if (retry) { > goto again; > printk("completing transaction\n"); > @@ -271,7 +271,7 @@ void shutdown_blkfront(struct blkfront_dev *dev) > state = xenbus_read_integer(path); > while (err == NULL && state < XenbusStateClosing) > err = xenbus_wait_for_state_change(path, &state, &dev->events); > - if (err) free(err); > + free(err); > > if ((err = xenbus_switch_state(XBT_NIL, nodename, XenbusStateClosed)) != NULL) { > printk("shutdown_blkfront: error changing state to %d: %s\n", > @@ -281,7 +281,7 @@ void shutdown_blkfront(struct blkfront_dev *dev) > state = xenbus_read_integer(path); > while (state < XenbusStateClosed) { > err = xenbus_wait_for_state_change(path, &state, &dev->events); > - if (err) free(err); > + free(err); > } > > if ((err = xenbus_switch_state(XBT_NIL, nodename, XenbusStateInitialising)) != NULL) { > @@ -294,16 +294,16 @@ void shutdown_blkfront(struct blkfront_dev *dev) > err = xenbus_wait_for_state_change(path, &state, &dev->events); > > close: > - if (err) free(err); > + free(err); > err2 = xenbus_unwatch_path_token(XBT_NIL, path, path); > - if (err2) free(err2); > + free(err2); > > snprintf(nodename, sizeof(nodename), "%s/ring-ref", dev->nodename); > err2 = xenbus_rm(XBT_NIL, nodename); > - if (err2) free(err2); > + free(err2); > snprintf(nodename, sizeof(nodename), "%s/event-channel", dev->nodename); > err2 = xenbus_rm(XBT_NIL, nodename); > - if (err2) free(err2); > + free(err2); > > if (!err) > free_blkfront(dev); > diff --git a/extras/mini-os/console/xenbus.c b/extras/mini-os/console/xenbus.c > index b317114..1c9a590 100644 > --- a/extras/mini-os/console/xenbus.c > +++ b/extras/mini-os/console/xenbus.c > @@ -32,7 +32,7 @@ void free_consfront(struct consfront_dev *dev) > state = xenbus_read_integer(path); > while (err == NULL && state < XenbusStateClosing) > err = xenbus_wait_for_state_change(path, &state, &dev->events); > - if (err) free(err); > + free(err); > > if ((err = xenbus_switch_state(XBT_NIL, nodename, XenbusStateClosed)) != NULL) { > printk("free_consfront: error changing state to %d: %s\n", > @@ -41,9 +41,9 @@ void free_consfront(struct consfront_dev *dev) > } > > close: > - if (err) free(err); > + free(err); > err = xenbus_unwatch_path_token(XBT_NIL, path, path); > - if (err) free(err); > + free(err); > > mask_evtchn(dev->evtchn); > unbind_evtchn(dev->evtchn); > @@ -134,7 +134,7 @@ again: > > > err = xenbus_transaction_end(xbt, 0, &retry); > - if (err) free(err); > + free(err); > if (retry) { > goto again; > printk("completing transaction\n"); > diff --git a/extras/mini-os/fbfront.c b/extras/mini-os/fbfront.c > index a276193..1e01513 100644 > --- a/extras/mini-os/fbfront.c > +++ b/extras/mini-os/fbfront.c > @@ -131,7 +131,7 @@ again: > } > > err = xenbus_transaction_end(xbt, 0, &retry); > - if (err) free(err); > + free(err); > if (retry) { > goto again; > printk("completing transaction\n"); > @@ -171,7 +171,7 @@ done: > err = xenbus_wait_for_state_change(path, &state, &dev->events); > if (state != XenbusStateConnected) { > printk("backend not available, state=%d\n", state); > - if (err) free(err); > + free(err); > err = xenbus_unwatch_path_token(XBT_NIL, path, path); > goto error; > } > @@ -256,7 +256,7 @@ void shutdown_kbdfront(struct kbdfront_dev *dev) > state = xenbus_read_integer(path); > while (err == NULL && state < XenbusStateClosing) > err = xenbus_wait_for_state_change(path, &state, &dev->events); > - if (err) free(err); > + free(err); > > if ((err = xenbus_switch_state(XBT_NIL, nodename, XenbusStateClosed)) != NULL) { > printk("shutdown_kbdfront: error changing state to %d: %s\n", > @@ -266,7 +266,7 @@ void shutdown_kbdfront(struct kbdfront_dev *dev) > state = xenbus_read_integer(path); > while (state < XenbusStateClosed) { > err = xenbus_wait_for_state_change(path, &state, &dev->events); > - if (err) free(err); > + free(err); > } > > if ((err = xenbus_switch_state(XBT_NIL, nodename, XenbusStateInitialising)) != NULL) { > @@ -279,19 +279,19 @@ void shutdown_kbdfront(struct kbdfront_dev *dev) > err = xenbus_wait_for_state_change(path, &state, &dev->events); > > close_kbdfront: > - if (err) free(err); > + free(err); > err2 = xenbus_unwatch_path_token(XBT_NIL, path, path); > - if (err2) free(err2); > + free(err2); > > snprintf(nodename, sizeof(nodename), "%s/page-ref", dev->nodename); > err2 = xenbus_rm(XBT_NIL, nodename); > - if (err2) free(err2); > + free(err2); > snprintf(nodename, sizeof(nodename), "%s/event-channel", dev->nodename); > err2 = xenbus_rm(XBT_NIL, nodename); > - if (err2) free(err2); > + free(err2); > snprintf(nodename, sizeof(nodename), "%s/request-abs-pointer", dev->nodename); > err2 = xenbus_rm(XBT_NIL, nodename); > - if (err2) free(err2); > + free(err2); > > if (!err) > free_kbdfront(dev); > @@ -498,7 +498,7 @@ again: > } > > err = xenbus_transaction_end(xbt, 0, &retry); > - if (err) free(err); > + free(err); > if (retry) { > goto again; > printk("completing transaction\n"); > @@ -538,7 +538,7 @@ done: > err = xenbus_wait_for_state_change(path, &state, &dev->events); > if (state != XenbusStateConnected) { > printk("backend not available, state=%d\n", state); > - if (err) free(err); > + free(err); > err = xenbus_unwatch_path_token(XBT_NIL, path, path); > goto error; > } > @@ -654,7 +654,7 @@ void shutdown_fbfront(struct fbfront_dev *dev) > state = xenbus_read_integer(path); > while (err == NULL && state < XenbusStateClosing) > err = xenbus_wait_for_state_change(path, &state, &dev->events); > - if (err) free(err); > + free(err); > > if ((err = xenbus_switch_state(XBT_NIL, nodename, XenbusStateClosed)) != NULL) { > printk("shutdown_fbfront: error changing state to %d: %s\n", > @@ -664,7 +664,7 @@ void shutdown_fbfront(struct fbfront_dev *dev) > state = xenbus_read_integer(path); > if (state < XenbusStateClosed) { > err = xenbus_wait_for_state_change(path, &state, &dev->events); > - if (err) free(err); > + free(err); > } > > if ((err = xenbus_switch_state(XBT_NIL, nodename, XenbusStateInitialising)) != NULL) { > @@ -677,22 +677,22 @@ void shutdown_fbfront(struct fbfront_dev *dev) > err = xenbus_wait_for_state_change(path, &state, &dev->events); > > close_fbfront: > - if (err) free(err); > + free(err); > err2 = xenbus_unwatch_path_token(XBT_NIL, path, path); > - if (err2) free(err2); > + free(err2); > > snprintf(nodename, sizeof(nodename), "%s/page-ref", dev->nodename); > err2 = xenbus_rm(XBT_NIL, nodename); > - if (err2) free(err2); > + free(err2); > snprintf(nodename, sizeof(nodename), "%s/event-channel", dev->nodename); > err2 = xenbus_rm(XBT_NIL, nodename); > - if (err2) free(err2); > + free(err2); > snprintf(nodename, sizeof(nodename), "%s/protocol", dev->nodename); > err2 = xenbus_rm(XBT_NIL, nodename); > - if (err2) free(err2); > + free(err2); > snprintf(nodename, sizeof(nodename), "%s/feature-update", dev->nodename); > err2 = xenbus_rm(XBT_NIL, nodename); > - if (err2) free(err2); > + free(err2); > > if (!err) > free_fbfront(dev); > diff --git a/extras/mini-os/kernel.c b/extras/mini-os/kernel.c > index 24fa25c..ea409f4 100644 > --- a/extras/mini-os/kernel.c > +++ b/extras/mini-os/kernel.c > @@ -85,9 +85,9 @@ static void shutdown_thread(void *p) > xenbus_wait_for_watch(&events); > } > err = xenbus_unwatch_path_token(XBT_NIL, path, token); > - if (err) free(err); > + free(err); > err = xenbus_write(XBT_NIL, path, ""); > - if (err) free(err); > + free(err); > printk("Shutting down (%s)\n", shutdown); > > if (!strcmp(shutdown, "poweroff")) > diff --git a/extras/mini-os/lib/sys.c b/extras/mini-os/lib/sys.c > index cfbdc90..13e7e59 100644 > --- a/extras/mini-os/lib/sys.c > +++ b/extras/mini-os/lib/sys.c > @@ -1156,8 +1156,7 @@ LWIP_STUB(int, getsockname, (int s, struct sockaddr *name, socklen_t *namelen), > static char *syslog_ident; > void openlog(const char *ident, int option, int facility) > { > - if (syslog_ident) > - free(syslog_ident); > + free(syslog_ident); > syslog_ident = strdup(ident); > } > > diff --git a/extras/mini-os/netfront.c b/extras/mini-os/netfront.c > index 3a5be64..44c3995 100644 > --- a/extras/mini-os/netfront.c > +++ b/extras/mini-os/netfront.c > @@ -412,7 +412,7 @@ again: > } > > err = xenbus_transaction_end(xbt, 0, &retry); > - if (err) free(err); > + free(err); > if (retry) { > goto again; > printk("completing transaction\n"); > @@ -525,7 +525,7 @@ void shutdown_netfront(struct netfront_dev *dev) > state = xenbus_read_integer(path); > while (err == NULL && state < XenbusStateClosing) > err = xenbus_wait_for_state_change(path, &state, &dev->events); > - if (err) free(err); > + free(err); > > if ((err = xenbus_switch_state(XBT_NIL, nodename, XenbusStateClosed)) != NULL) { > printk("shutdown_netfront: error changing state to %d: %s\n", > @@ -535,7 +535,7 @@ void shutdown_netfront(struct netfront_dev *dev) > state = xenbus_read_integer(path); > while (state < XenbusStateClosed) { > err = xenbus_wait_for_state_change(path, &state, &dev->events); > - if (err) free(err); > + free(err); > } > > if ((err = xenbus_switch_state(XBT_NIL, nodename, XenbusStateInitialising)) != NULL) { > @@ -548,22 +548,22 @@ void shutdown_netfront(struct netfront_dev *dev) > err = xenbus_wait_for_state_change(path, &state, &dev->events); > > close: > - if (err) free(err); > + free(err); > err2 = xenbus_unwatch_path_token(XBT_NIL, path, path); > - if (err2) free(err2); > + free(err2); > > snprintf(nodename, sizeof(nodename), "%s/tx-ring-ref", dev->nodename); > err2 = xenbus_rm(XBT_NIL, nodename); > - if (err2) free(err2); > + free(err2); > snprintf(nodename, sizeof(nodename), "%s/rx-ring-ref", dev->nodename); > err2 = xenbus_rm(XBT_NIL, nodename); > - if (err2) free(err2); > + free(err2); > snprintf(nodename, sizeof(nodename), "%s/event-channel", dev->nodename); > err2 = xenbus_rm(XBT_NIL, nodename); > - if (err2) free(err2); > + free(err2); > snprintf(nodename, sizeof(nodename), "%s/request-rx-copy", dev->nodename); > err2 = xenbus_rm(XBT_NIL, nodename); > - if (err2) free(err2); > + free(err2); > > if (!err) > free_netfront(dev); > diff --git a/extras/mini-os/pcifront.c b/extras/mini-os/pcifront.c > index 7838021..16a4b49 100644 > --- a/extras/mini-os/pcifront.c > +++ b/extras/mini-os/pcifront.c > @@ -212,7 +212,7 @@ again: > } > > err = xenbus_transaction_end(xbt, 0, &retry); > - if (err) free(err); > + free(err); > if (retry) { > goto again; > printk("completing transaction\n"); > @@ -251,7 +251,7 @@ done: > err = xenbus_wait_for_state_change(path, &state, &dev->events); > if (state != XenbusStateConnected) { > printk("backend not avalable, state=%d\n", state); > - if (err) free(err); > + free(err); > err = xenbus_unwatch_path_token(XBT_NIL, path, path); > goto error; > } > @@ -342,7 +342,7 @@ void shutdown_pcifront(struct pcifront_dev *dev) > state = xenbus_read_integer(path); > while (err == NULL && state < XenbusStateClosing) > err = xenbus_wait_for_state_change(path, &state, &dev->events); > - if (err) free(err); > + free(err); > > if ((err = xenbus_switch_state(XBT_NIL, nodename, XenbusStateClosed)) != NULL) { > printk("shutdown_pcifront: error changing state to %d: %s\n", > @@ -365,16 +365,16 @@ void shutdown_pcifront(struct pcifront_dev *dev) > err = xenbus_wait_for_state_change(path, &state, &dev->events); > > close_pcifront: > - if (err) free(err); > + free(err); > err2 = xenbus_unwatch_path_token(XBT_NIL, path, path); > - if (err2) free(err2); > + free(err2); > > snprintf(nodename, sizeof(nodename), "%s/info-ref", dev->nodename); > err2 = xenbus_rm(XBT_NIL, nodename); > - if (err2) free(err2); > + free(err2); > snprintf(nodename, sizeof(nodename), "%s/event-channel", dev->nodename); > err2 = xenbus_rm(XBT_NIL, nodename); > - if (err2) free(err2); > + free(err2); > > if (!err) > free_pcifront(dev); > diff --git a/tools/flask/utils/loadpolicy.c b/tools/flask/utils/loadpolicy.c > index f347b97..2edcf97 100644 > --- a/tools/flask/utils/loadpolicy.c > +++ b/tools/flask/utils/loadpolicy.c > @@ -108,8 +108,7 @@ int main (int argCnt, const char *args[]) > } > > done: > - if ( polMemCp ) > - free(polMemCp); > + free(polMemCp); > if ( polMem ) > { > ret = munmap(polMem, info.st_size); > diff --git a/tools/libxc/xc_compression.c b/tools/libxc/xc_compression.c > index 89f1be7..8f0b89d 100644 > --- a/tools/libxc/xc_compression.c > +++ b/tools/libxc/xc_compression.c > @@ -457,16 +457,11 @@ void xc_compression_free_context(xc_interface *xch, comp_ctx *ctx) > { > if (!ctx) return; > > - if (ctx->inputbuf) > - free(ctx->inputbuf); > - if (ctx->sendbuf_pfns) > - free(ctx->sendbuf_pfns); > - if (ctx->cache_base) > - free(ctx->cache_base); > - if (ctx->pfn2cache) > - free(ctx->pfn2cache); > - if (ctx->cache) > - free(ctx->cache); > + free(ctx->inputbuf); > + free(ctx->sendbuf_pfns); > + free(ctx->cache_base); > + free(ctx->pfn2cache); > + free(ctx->cache); > free(ctx); > } > > diff --git a/tools/libxc/xc_core_x86.c b/tools/libxc/xc_core_x86.c > index a9681f7..e328dcf 100644 > --- a/tools/libxc/xc_core_x86.c > +++ b/tools/libxc/xc_core_x86.c > @@ -180,11 +180,9 @@ out: > if ( live_p2m_frame_list ) > munmap(live_p2m_frame_list, P2M_FLL_ENTRIES * PAGE_SIZE); > > - if ( p2m_frame_list_list ) > - free(p2m_frame_list_list); > + free(p2m_frame_list_list); > > - if ( p2m_frame_list ) > - free(p2m_frame_list); > + free(p2m_frame_list); > > errno = err; > return ret; > diff --git a/tools/libxc/xc_domain_save.c b/tools/libxc/xc_domain_save.c > index fbc15e9..673d1e9 100644 > --- a/tools/libxc/xc_domain_save.c > +++ b/tools/libxc/xc_domain_save.c > @@ -773,11 +773,9 @@ static xen_pfn_t *map_and_save_p2m_table(xc_interface *xch, > if ( live_p2m_frame_list ) > munmap(live_p2m_frame_list, P2M_FLL_ENTRIES * PAGE_SIZE); > > - if ( p2m_frame_list_list ) > - free(p2m_frame_list_list); > + free(p2m_frame_list_list); > > - if ( p2m_frame_list ) > - free(p2m_frame_list); > + free(p2m_frame_list); > > return success ? p2m : NULL; > } > diff --git a/tools/libxc/xc_gnttab.c b/tools/libxc/xc_gnttab.c > index a8cc381..79dab40 100644 > --- a/tools/libxc/xc_gnttab.c > +++ b/tools/libxc/xc_gnttab.c > @@ -124,8 +124,7 @@ static void *_gnttab_map_table(xc_interface *xch, int domid, int *gnt_num) > err: > if ( frame_list ) > xc_hypercall_buffer_free(xch, frame_list); > - if ( pfn_list ) > - free(pfn_list); > + free(pfn_list); > > return gnt; > } > diff --git a/tools/libxc/xc_offline_page.c b/tools/libxc/xc_offline_page.c > index 8195efb..0b4cdf9 100644 > --- a/tools/libxc/xc_offline_page.c > +++ b/tools/libxc/xc_offline_page.c > @@ -626,14 +626,11 @@ failed: > } > } > > - if (mmu) > - free(mmu); > + free(mmu); > > - if (old_ptes.entries) > - free(old_ptes.entries); > + free(old_ptes.entries); > > - if (backup) > - free(backup); > + free(backup); > > if (gnttab_v1) > munmap(gnttab_v1, gnt_num / (PAGE_SIZE/sizeof(grant_entry_v1_t))); > diff --git a/tools/libxl/libxl_event.c b/tools/libxl/libxl_event.c > index a5c52bc..c5e9426 100644 > --- a/tools/libxl/libxl_event.c > +++ b/tools/libxl/libxl_event.c > @@ -578,8 +578,7 @@ int libxl__ev_xswatch_register(libxl__gc *gc, libxl__ev_xswatch *w, > out_rc: > if (use) > LIBXL_SLIST_INSERT_HEAD(&CTX->watch_freeslots, use, empty); > - if (path_copy) > - free(path_copy); > + free(path_copy); > CTX_UNLOCK; > return rc; > } > diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c > index f681f3a..7e825ee 100644 > --- a/tools/libxl/libxl_qmp.c > +++ b/tools/libxl/libxl_qmp.c > @@ -406,12 +406,10 @@ static void qmp_close(libxl__qmp_handler *qmp) > > close(qmp->qmp_fd); > LIBXL_STAILQ_FOREACH(pp, &qmp->callback_list, next) { > - if (tmp) > - free(tmp); > + free(tmp); > tmp = pp; > } > - if (tmp) > - free(tmp); > + free(tmp); > } > > static int qmp_next(libxl__gc *gc, libxl__qmp_handler *qmp) > diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c > index 1bcac7e..682f874 100644 > --- a/tools/libxl/libxl_utils.c > +++ b/tools/libxl/libxl_utils.c > @@ -355,7 +355,7 @@ int libxl_read_file_contents(libxl_ctx *ctx, const char *filename, > e = errno; > assert(e != ENOENT); > if (f) fclose(f); > - if (data) free(data); > + free(data); > return e; > } > > diff --git a/tools/memshr/bidir-hash.c b/tools/memshr/bidir-hash.c > index bed8179..3d34637 100644 > --- a/tools/memshr/bidir-hash.c > +++ b/tools/memshr/bidir-hash.c > @@ -208,8 +208,8 @@ static void free_buckets(struct __hash *h, > struct bucket *buckets, > struct bucket_lock *bucket_locks) > { > - if(buckets) free(buckets); > - if(bucket_locks) free(bucket_locks); > + free(buckets); > + free(bucket_locks); > } > > static int max_entries(struct __hash *h) > diff --git a/tools/misc/gtraceview.c b/tools/misc/gtraceview.c > index d8b4589..cf9287c 100644 > --- a/tools/misc/gtraceview.c > +++ b/tools/misc/gtraceview.c > @@ -959,8 +959,7 @@ int time_mode_rebuild(uint64_t start_time, uint64_t time_scale) > state = malloc(sizeof(struct state) * number); > if (!state) > return 1; > - if (this->state) > - free(this->state); > + free(this->state); > this->state = state; > this->width = 9; > this->row = 0; > diff --git a/tools/tests/xen-access/xen-access.c b/tools/tests/xen-access/xen-access.c > index 49195a8..b00c05a 100644 > --- a/tools/tests/xen-access/xen-access.c > +++ b/tools/tests/xen-access/xen-access.c > @@ -219,10 +219,8 @@ int xenaccess_teardown(xc_interface *xch, xenaccess_t *xenaccess) > } > xenaccess->xc_handle = NULL; > > - if ( xenaccess->platform_info ) > - free(xenaccess->platform_info); > - if ( xenaccess->domain_info ) > - free(xenaccess->domain_info); > + free(xenaccess->platform_info); > + free(xenaccess->domain_info); > free(xenaccess); > > return 0; > diff --git a/tools/xenbackendd/xenbackendd.c b/tools/xenbackendd/xenbackendd.c > index 5381a2a..f1eb1f5 100644 > --- a/tools/xenbackendd/xenbackendd.c > +++ b/tools/xenbackendd/xenbackendd.c > @@ -291,8 +291,7 @@ main(int argc, char * const argv[]) > > switch(type) { > case DEVTYPE_VIF: > - if (s) > - free(s); > + free(s); > snprintf(buf, sizeof(buf), "%s/script", > vec[XS_WATCH_PATH]); > s = xs_read(xs, XBT_NULL, buf, 0); > @@ -314,8 +313,7 @@ main(int argc, char * const argv[]) > } > > next2: > - if (s) > - free(s); > + free(s); > free(sstate); > > next1: