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

kvs: testing, fix use-after-free, streamline python bindings #823

Merged
merged 11 commits into from Sep 27, 2016

Conversation

Projects
None yet
4 participants
@garlick
Copy link
Member

garlick commented Sep 27, 2016

This PR contains some misc. hopefully non-controversial improvements culled from PR #820 which was withdrawn. There are three batches here:

  • testing: unit test for json_dirent.[ch], new dtree test for populating kvs namespace
  • memory corruption: fix two use-after-free bugs in kvs module, protect flux_rpc_t with magic cookie, get valgrind test working again
  • api: add format attributes on varargs kvs functions, update a few callers; drop deprecated kvs interfaces from python bindings

garlick added some commits Sep 15, 2016

libflux/rpc: add "magic" assertion
Tracking down a memory corruption problem which would
have been found long ago had these assertions been present
in the code.
modules/kvs: fix use-after-free mem corruption
The store() function was trying to free a flux_rpc_t twice,
resulting in a use-after-free as flux_rpc_destroy tried
to decrement the rpc's usecount.
modules/kvs: add format attribute
This catches "format not a string literal" errors when
printf-style arguments are misused.
modules/kvs: fix use-after-free
Fix code in load() that would destroy flux_rpc_t twice.
test/kvs: add dtree test
Add a program to create a H x W key namespace hierarchy.
That is, create W^H keys such that directories are H levels
deep and W entries wide.  The keys are created in one commit.

Use this in t1000-kvs-basic.t sharness test to create a
hierarchical key space and walk it with "flux kvs dir".

@garlick garlick added the review label Sep 27, 2016

@coveralls

This comment has been minimized.

Copy link

coveralls commented Sep 27, 2016

Coverage Status

Coverage increased (+0.02%) to 75.263% when pulling 24b4b52 on garlick:misc_fixes into bc651df on flux-framework:master.

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Sep 27, 2016

Current coverage is 74.94% (diff: 91.66%)

Merging #823 into master will increase coverage by 0.02%

@@             master       #823   diff @@
==========================================
  Files           145        145          
  Lines         24918      24935    +17   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits          18670      18688    +18   
+ Misses         6248       6247     -1   
  Partials          0          0          
Diff Coverage File Path
••••• 50% src/bindings/lua/kvs-lua.c
••••••••• 92% src/common/libflux/rpc.c
•••••••••• 100% src/bindings/lua/flux-lua.c
•••••••••• 100% src/modules/kvs/kvs.c
•••••••••• 100% src/modules/kvs/json_dirent.c

Powered by Codecov. Last update bc651df...24b4b52

@grondo

This comment has been minimized.

Copy link
Contributor

grondo commented Sep 27, 2016

Looks good to me! anything else to do here?

@garlick

This comment has been minimized.

Copy link
Member Author

garlick commented Sep 27, 2016

Nope, I think it can go in.

@grondo grondo merged commit 927ac17 into flux-framework:master Sep 27, 2016

4 checks passed

codecov/patch 91.66% of diff hit (target 74.92%)
Details
codecov/project 74.94% (+0.02%) compared to bc651df
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.02%) to 75.263%
Details

@grondo grondo removed the review label Sep 27, 2016

@garlick garlick deleted the garlick:misc_fixes branch Sep 27, 2016

@garlick

This comment has been minimized.

Copy link
Member Author

garlick commented Sep 27, 2016

Thanks!

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.