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

API: add const to message payload accessor functions #1212

Merged
merged 6 commits into from Sep 27, 2017

Conversation

Projects
None yet
4 participants
@garlick
Copy link
Member

garlick commented Sep 27, 2017

As noted in #1211, flux_msg_get_payload() and all the functions based on it should use const on the void **data paramter, since the payload belongs to the message (also const).

New prototypes:

int flux_msg_get_payload (const flux_msg_t *msg, int *flags,
                          const void **buf, int *size);

int flux_request_decode_raw (const flux_msg_t *msg, const char **topic,
                             const void **data, int *len);

int flux_response_decode_raw (const flux_msg_t *msg, const char **topic,
                              const void **data, int *len);

int flux_content_load_get (flux_future_t *f, const void **buf, int *len);

int flux_rpc_get_raw (flux_future_t *f, const void **data, int *len);

int flux_mrpc_get_raw (flux_mrpc_t *mrpc, const void **data, int *len);

All the internal code that touches these functions was also updated.

Man pages were updated.

@coveralls

This comment has been minimized.

Copy link

coveralls commented Sep 27, 2017

Coverage Status

Coverage decreased (-0.01%) to 78.667% when pulling 309efd5 on garlick:const_payload into 6486563 on flux-framework:master.

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Sep 27, 2017

Codecov Report

Merging #1212 into master will decrease coverage by 0.04%.
The diff coverage is 93.75%.

@@            Coverage Diff             @@
##           master    #1212      +/-   ##
==========================================
- Coverage   78.26%   78.22%   -0.05%     
==========================================
  Files         158      158              
  Lines       29139    29139              
==========================================
- Hits        22806    22794      -12     
- Misses       6333     6345      +12
Impacted Files Coverage Δ
src/modules/kvs/kvs.c 63.67% <ø> (ø) ⬆️
src/common/libflux/mrpc.c 86.66% <0%> (ø) ⬆️
src/common/libflux/content.c 90% <100%> (ø) ⬆️
src/broker/log.c 72.89% <100%> (ø) ⬆️
src/common/libutil/readall.c 81.81% <100%> (ø) ⬆️
src/cmd/builtin/content.c 73.87% <100%> (ø) ⬆️
src/common/libflux/message.c 81.13% <100%> (-0.36%) ⬇️
src/modules/content-sqlite/content-sqlite.c 56.75% <100%> (ø) ⬆️
src/common/libflux/rpc.c 94.21% <100%> (ø) ⬆️
src/common/libflux/response.c 84.55% <100%> (ø) ⬆️
... and 13 more
@chu11

This comment has been minimized.

Copy link
Contributor

chu11 commented Sep 27, 2017

a quick skim, it looks good to me. Did notice this typo in the commit message.

Problem: flux_msg_get_payload() accepts a a void **

a a void **

@garlick

This comment has been minimized.

Copy link
Member Author

garlick commented Sep 27, 2017

Ooops! I'll fix.

@garlick garlick force-pushed the garlick:const_payload branch from 309efd5 to 9f47de9 Sep 27, 2017

@garlick

This comment has been minimized.

Copy link
Member Author

garlick commented Sep 27, 2017

Forced a push with the commit message typo fix.

@chu11

This comment has been minimized.

Copy link
Contributor

chu11 commented Sep 27, 2017

once it passes will hit the button

@chu11

This comment has been minimized.

Copy link
Contributor

chu11 commented Sep 27, 2017

Actually one more typo, is "writeall" supposed to be "write_all"?

@garlick

This comment has been minimized.

Copy link
Member Author

garlick commented Sep 27, 2017

Argh! fixing...

@garlick garlick force-pushed the garlick:const_payload branch from 9f47de9 to 32e96cb Sep 27, 2017

garlick added some commits Sep 27, 2017

libflux: convert some params to const
Problem: flux_msg_get_payload() accepts a void **
argument that is set to point to the message payload,
but the payload belongs to the const flux_msg_t *
passed in as another argument.

Make the payload pointer 'const'.

The following functions, based on the above, are
similarly updated:

flux_request_decode_raw()
flux_response_decode_raw()
flux_rpc_get_raw()
flux_mrpc_get_raw()
flux_content_load_get()

Finally, all users of the above functions are updated.

And a libutil function write_all() parameter was changed
to const as well, since flux-content was using it on
payload returned from flux_content_load_get(), and
anyway it was appropriate there.

Fixes #1211
doc/flux_content_load(3): update func prototype
Update for flux_content_load_get() prototype change.
doc/flux_rpc(3): update func prototype
Update for flux_rpc_get_raw() prototype change.
doc/flux_response_decode(3): update func prototype
Update for flux_response_decode_raw() prototype change.
doc/flux_request_decode(3): update func prototype
Update for flux_request_decode_raw() prototype change
doc/flux_mrpc(3): update func prototype
Update for flux_mrpc_get_raw() prototype change.
@coveralls

This comment has been minimized.

Copy link

coveralls commented Sep 27, 2017

Coverage Status

Coverage increased (+0.09%) to 78.772% when pulling 9f47de9 on garlick:const_payload into 6486563 on flux-framework:master.

@coveralls

This comment has been minimized.

Copy link

coveralls commented Sep 27, 2017

Coverage Status

Coverage decreased (-0.04%) to 78.638% when pulling 32e96cb on garlick:const_payload into 6486563 on flux-framework:master.

@chu11 chu11 merged commit aa3cac0 into flux-framework:master Sep 27, 2017

4 checks passed

codecov/patch 93.75% of diff hit (target 78.26%)
Details
codecov/project Absolute coverage decreased by -0.04% but relative coverage increased by +15.48% compared to 6486563
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.04%) to 78.638%
Details
@garlick

This comment has been minimized.

Copy link
Member Author

garlick commented Sep 27, 2017

Thanks!

@garlick garlick deleted the garlick:const_payload branch Sep 28, 2017

@grondo grondo referenced this pull request May 10, 2018

Closed

0.9.0 Release #1479

chu11 added a commit to chu11/flux-core that referenced this pull request Jul 27, 2018

libflux/future: convert void * to const void **
In flux_future_get, convert void * parameter to void ** parameter,
consistent to fixes done in flux-framework#1144 and flux-framework#1212.

Fixes flux-framework#1602

chu11 added a commit to chu11/flux-core that referenced this pull request Jul 27, 2018

libflux/future: convert void * to const void **
In flux_future_get, convert void * parameter to void ** parameter,
consistent to fixes done in flux-framework#1144 and flux-framework#1212.

Fixes flux-framework#1602
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.