Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fixes for minor issues detected by Coverity #876

Merged
merged 10 commits into from Oct 26, 2016

Conversation

Projects
None yet
4 participants
@grondo
Copy link
Contributor

grondo commented Oct 25, 2016

Nothing major here, but a series of minor fixups for issues flagged by Coverity.
Most of these are in error handling so we were unlikely to hit them.

A few of these were cleaned up by moving from raw JSON handling through json-c to use of flux_rpcf, flux_rpc_getf and flux_respondf etc.

@grondo grondo added the review label Oct 25, 2016

@coveralls

This comment has been minimized.

Copy link

coveralls commented Oct 25, 2016

Coverage Status

Coverage decreased (-0.06%) to 75.649% when pulling 5e68733 on grondo:coverity-fixes into 342b127 on flux-framework:master.

@garlick

This comment has been minimized.

Copy link
Member

garlick commented Oct 25, 2016

Looks good! There was one travis failure on the intermittent t0016-cron-faketime.t test, so I restarted it.

@@ -198,7 +198,7 @@ static char *escape_and_join_kvs_path (const char *base,
suffix = escape_kvs_key (suffix);

if (!ret_str || !strlen (ret_str))
ret_str = suffix;
ret_str = strdup (suffix);

This comment has been minimized.

@garlick

garlick Oct 25, 2016

Member

should that be an xstrdup() to match the xasprintf() on the other leg of the conditional?

grondo added some commits Oct 22, 2016

resource-hwloc: fix use-after-free
Fix use-after-free in escape_and_join_kvs_path() detected by
Coverity.

CID-129813
modules/cron: fix use-after-free in cron_entry_create
Coverity detected use after free in cron_entry_create() when
allocation of a cron id fails. The entry, e, should be set
to NULL when memory is freed.

CID-149369
libpmi/simple_server: fix minor leak in mcmd_begin()
Coverity detected unlikely leak in simple_server.c:mcmd_begin()
on zlist_new() failure.

CID-151518
libjsc: fix leak in fixup_newjob_event
Coverity detected leak in fixup_newjob_event(), local json_object
jcb is never freed.

CID-153483
modules/kvs: fix potential leak in kvs_mkdir()
Fix Coverity detected leak of `val` in kvs_mkdir().

CID-153484
modules/cron: fix potential leak in io_handler
Fix potential leak in task.c:io_handler(), zio_json_decode()
may allocate memory in data but still return nonzero.
libutil: set dst to NULL if base64_decode fails
If base64_decode fails in base64_json_decode(), set dst pointer
to NULL (and thus return by reference pointer to NULL) after
freeing it to avoid potential for use-after-free in callers.
broker: use flux_respondf in cmb_write/signal_cb
Simplify the broker subprocess write and signal handlers by using
flux_respondf() instead of custom JSON encoding. This removes
the need for test and free at function return for the response
JSON object.

CID-153490, CID-153488
wreck: fix resource leak in update_job_state
Fix Coverity detected memory leak in update_job_state() when
asprintf() fails.

CID-153482
wreck: simplify JSON handling in job module
Convert, where appropriate, job module functions to use flux_rpcf,
flux_rpc_getf, and flux_respondf to greatly simplify JSON handling,
remove some possible memory leaks, etc.
@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Oct 26, 2016

Current coverage is 72.06% (diff: 50.00%)

Merging #876 into master will decrease coverage by 0.03%

@@             master       #876   diff @@
==========================================
  Files           156        156          
  Lines         26957      26942    -15   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
- Hits          19435      19415    -20   
- Misses         7522       7527     +5   
  Partials          0          0          
Diff Coverage File Path
0% src/modules/cron/cron.c
0% src/common/libpmi/simple_server.c
0% src/modules/resource-hwloc/resource.c
0% src/common/libutil/base64_json.c
0% src/modules/cron/task.c
••• 33% src/modules/wreck/wrexecd.c
••••• 50% src/broker/broker.c
•••••• 61% src/modules/wreck/job.c
••••••• 75% src/modules/kvs/libkvs.c
•••••••••• 100% src/modules/libjsc/jstatctl.c

Powered by Codecov. Last update 342b127...319ec53

@grondo grondo force-pushed the grondo:coverity-fixes branch from 5e68733 to 319ec53 Oct 26, 2016

@coveralls

This comment has been minimized.

Copy link

coveralls commented Oct 26, 2016

Coverage Status

Coverage decreased (-0.03%) to 75.672% when pulling 319ec53 on grondo:coverity-fixes into 342b127 on flux-framework:master.

@garlick garlick merged commit 9cea79b into flux-framework:master Oct 26, 2016

2 of 4 checks passed

codecov/patch 50.00% of diff hit (target 72.09%)
Details
codecov/project 72.06% (-0.04%) compared to 342b127
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.03%) to 75.672%
Details

@garlick garlick removed the review label Oct 26, 2016

@grondo

This comment has been minimized.

Copy link
Contributor Author

grondo commented Oct 26, 2016

Good point! Probably!

On Oct 25, 2016 4:57 PM, "Jim Garlick" notifications@github.com wrote:

@garlick commented on this pull request.

In src/modules/resource-hwloc/resource.c
#876 (review)
:

@@ -198,7 +198,7 @@ static char *escape_and_join_kvs_path (const char *base,
suffix = escape_kvs_key (suffix);

     if (!ret_str || !strlen (ret_str))
  •        ret_str = suffix;
    
  •        ret_str = strdup (suffix);
    

should that be an xstrdup() to match the xasprintf() on the other leg of
the conditional?


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#876 (review),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAtSUhiNOEeT5-XzkK_72WZJ414GNlUrks5q3pdQgaJpZM4KgmFp
.

@garlick garlick referenced this pull request Oct 26, 2016

Closed

0.5.0 release notes #879

@grondo grondo deleted the grondo:coverity-fixes branch Nov 3, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.