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

modules/kvs: normalize keys before use #1182

Merged
merged 3 commits into from Sep 6, 2017

Conversation

Projects
None yet
4 participants
@garlick
Copy link
Member

garlick commented Sep 6, 2017

This PR adds code to deal with extra path separators in keys.

Duplicate, leading, and trailing separators are removed, which addresses #1180 and #1173.

Sharness and unit tests are added.

garlick added some commits Sep 6, 2017

modules/kvs: add kvs_util_normalize_key()
Add a utility function kvs_util_normalize_key() which
- transforms consecutive path separators into one
- drops leading path separators
- drops trailing path separators
- preserves "." as such

Add unit tests.
modules/kvs: normalize keys before use
Insert key normalization in
1) lookup_create(), which covers "get" and "watch" cases
2) in commit_link_dirent() which covers "put"
3) in kvs_unwatch()

Fixes #1180
Fixes #1173
@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Sep 6, 2017

Codecov Report

Merging #1182 into master will decrease coverage by <.01%.
The diff coverage is 84.78%.

@@            Coverage Diff             @@
##           master    #1182      +/-   ##
==========================================
- Coverage   77.84%   77.84%   -0.01%     
==========================================
  Files         158      158              
  Lines       29209    29252      +43     
==========================================
+ Hits        22739    22770      +31     
- Misses       6470     6482      +12
Impacted Files Coverage Δ
src/modules/kvs/lookup.c 84.69% <100%> (ø) ⬆️
src/modules/kvs/kvs_util.c 87.75% <100%> (+10.83%) ⬆️
src/modules/kvs/kvs.c 63.16% <66.66%> (-0.72%) ⬇️
src/modules/kvs/commit.c 77.34% <75%> (-0.43%) ⬇️
src/common/libflux/keepalive.c 86.66% <0%> (-6.67%) ⬇️
src/common/libutil/blobref.c 94.59% <0%> (-1.36%) ⬇️
src/common/libkvs/kvs.c 65.08% <0%> (-0.72%) ⬇️
src/common/libutil/base64.c 95.07% <0%> (-0.71%) ⬇️
src/broker/overlay.c 71.67% <0%> (-0.35%) ⬇️
... and 7 more
@grondo

This comment has been minimized.

Copy link
Contributor

grondo commented Sep 6, 2017

Looks good to me! Just waiting for Travis to finish up

@grondo

This comment has been minimized.

Copy link
Contributor

grondo commented Sep 6, 2017

Eh, coveralls seems to be slow but we already have coverage report from codecov, so all looks good!

@grondo grondo merged commit 9c860d0 into flux-framework:master Sep 6, 2017

3 of 4 checks passed

coverage/coveralls Coverage pending from Coveralls.io
Details
codecov/patch 84.78% of diff hit (target 77.84%)
Details
codecov/project Absolute coverage decreased by -<.01% but relative coverage increased by +6.93% compared to 9dcd16a
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@garlick garlick deleted the garlick:kvs_norm_key branch Sep 6, 2017

@coveralls

This comment has been minimized.

Copy link

coveralls commented Sep 7, 2017

Coverage Status

Coverage increased (+0.009%) to 78.187% when pulling 4d3e195 on garlick:kvs_norm_key into 9dcd16a on flux-framework:master.

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.