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.

Sign up for GitHub

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

Jump to bottom

buffer.h: remove list::iterator_impl::advance(size_t) #28278

Merged
merged 1 commit into from
May 30, 2019
Merged

buffer.h: remove list::iterator_impl::advance(size_t) #28278

merged 1 commit into from
May 30, 2019

Conversation

tchaikov
Copy link
Contributor

quote from Radoslaw's comment

In general we could remove it as the buffer library tends to
disrespects size_t – list::_len and ptr::_len are both unsigned.
There are some exceptions like size_t buffer::ptr::get_offset() (used
altogether with ::advance in denc.h) but they deserve rework as
well.

and in this very case, we simply cast size_t to unsigned. so we
could silently trim a 64bits integer to 32bit on 64bits architecture.

so for better portability, we can just remove it. also, this function is
inlined, so the ABI is not changed.

Signed-off-by: Kefu Chai kchai@redhat.com

  • References tracker ticket
  • Updates documentation if necessary
  • Includes tests for new functionality or reproducer for bug

This was referenced May 29, 2019
Merged
Merged
@tchaikov tchaikov added the DNM label May 29, 2019
@tchaikov
Copy link
Contributor Author 

/home/jenkins-build/build/workspace/ceph-pull-requests/src/include/denc.h:1584:30: error: call of overloaded 'advance(size_t)' is ambiguous
     p.advance(cp.get_offset());
                              ^

@tchaikov tchaikov force-pushed the wip-buffer-remove-advance-size_t branch from 09375e2 to 1e315ec Compare May 29, 2019 08:52
@tchaikov tchaikov removed the DNM label May 29, 2019
@tchaikov tchaikov force-pushed the wip-buffer-remove-advance-size_t branch from 1e315ec to 03e857c Compare May 29, 2019 08:53
@tchaikov
Copy link
Contributor Author

changelog

  • remove void advance(int o) = delete which causes ambiguity
  • add a commit which silence the warning.

tchaikov
tchaikov commented May 29, 2019
View reviewed changes
src/include/blobhash.h Outdated Show resolved Hide resolved
@tchaikov
buffer.h: remove list::iterator_impl::advance(size_t)
a9f8b1a 
quote from Radoslaw's comment

> In general we could remove it as the `buffer` library tends to
> disrespects `size_t` – `list::_len` and `ptr::_len` are both
`unsigned`.
> There are some exceptions like `size_t buffer::ptr::get_offset()`
(used
> altogether with `::advance` in `denc.h`) but they deserve rework as
> well.

and in this very case, we simply cast `size_t` to `unsigned`. so we
could silently trim a 64bits integer to 32bit on 64bits architecture.

so for better portability, we can just remove it. also, this function is
inlined, so the ABI is not changed.

Signed-off-by: Kefu Chai <kchai@redhat.com>
@tchaikov tchaikov force-pushed the wip-buffer-remove-advance-size_t branch from 03e857c to a9f8b1a Compare May 29, 2019 15:55
@tchaikov
Copy link
Contributor Author

changelog

@tchaikov tchaikov merged commit b2019a1 into ceph:master May 30, 2019
@tchaikov
Copy link
Contributor Author

@tchaikov tchaikov deleted the wip-buffer-remove-advance-size_t branch May 30, 2019 13:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@javacruft javacruft javacruft approved these changes

@rzarzynski rzarzynski rzarzynski approved these changes

Assignees
No one assigned
Labels
cleanup common needs-qa wip-kefu-testing
Projects
None yet
Milestone
No milestone
3 participants
@tchaikov @javacruft @rzarzynski