Opened 9 years ago

Last modified 9 years ago

#8612 new Bugs

unused pointers in copying of handlers

Reported by: Richard <legalize@…> Owned by: chris_kohlhoff
Milestone: To Be Determined Component: asio
Version: Boost 1.52.0 Severity: Cosmetic
Keywords: Cc:

Description

boost/asio/detail/reactive_socket_recv_op.hpp:

 84  static void do_complete(io_service_impl* owner, operation* base,
 85      const boost::system::error_code& /*ec*/,
 86      std::size_t /*bytes_transferred*/)
 87  {
 88    // Take ownership of the handler object.
 89    reactive_socket_recv_op* o(static_cast<reactive_socket_recv_op*>(base));

CID 11125 (2): Unused pointer value (UNUSED_VALUE)
CID 10926 (#1 of 1): Unused pointer value (UNUSED_VALUE)
returned_pointer: Pointer "p.h" returned by "addressof(o->handler_)" is never used. 
 90    ptr p = { boost::addressof(o->handler_), o, o };
 91
 92    BOOST_ASIO_HANDLER_COMPLETION((o));
 93
 94    // Make a copy of the handler so that the memory can be deallocated before
 95    // the upcall is made. Even if we're not about to make an upcall, a
 96    // sub-object of the handler may be the true owner of the memory associated
 97    // with the handler. Consequently, a local copy of the handler is required
 98    // to ensure that any owning sub-object remains valid until after we have
 99    // deallocated the memory here.
100    detail::binder2<Handler, boost::system::error_code, std::size_t>
101      handler(o->handler_, o->ec_, o->bytes_transferred_);
102    p.h = boost::addressof(handler.handler_);
103

boost/asio/detail/reactive_socket_send_op.hpp:

 81  static void do_complete(io_service_impl* owner, operation* base,
 82      const boost::system::error_code& /*ec*/,
 83      std::size_t /*bytes_transferred*/)
 84  {
 85    // Take ownership of the handler object.
 86    reactive_socket_send_op* o(static_cast<reactive_socket_send_op*>(base));

CID 11126 (4): Unused pointer value (UNUSED_VALUE)
CID 11126 (4): Unused pointer value (UNUSED_VALUE)
CID 10927 (#2 of 2): Unused pointer value (UNUSED_VALUE)
CID 10927 (#1 of 2): Unused pointer value (UNUSED_VALUE)
returned_pointer: Pointer "p.h" returned by "addressof(o->handler_)" is never used. 
 87    ptr p = { boost::addressof(o->handler_), o, o };
 88
 89    BOOST_ASIO_HANDLER_COMPLETION((o));
 90
 91    // Make a copy of the handler so that the memory can be deallocated before
 92    // the upcall is made. Even if we're not about to make an upcall, a
 93    // sub-object of the handler may be the true owner of the memory associated
 94    // with the handler. Consequently, a local copy of the handler is required
 95    // to ensure that any owning sub-object remains valid until after we have
 96    // deallocated the memory here.
 97    detail::binder2<Handler, boost::system::error_code, std::size_t>
 98      handler(o->handler_, o->ec_, o->bytes_transferred_);
 99    p.h = boost::addressof(handler.handler_);

boost/asio/detail/wait_handler.cpp:

43  static void do_complete(io_service_impl* owner, operation* base,
44      const boost::system::error_code& /*ec*/,
45      std::size_t /*bytes_transferred*/)
46  {
47    // Take ownership of the handler object.
48    wait_handler* h(static_cast<wait_handler*>(base));

CID 11127 (2): Unused pointer value (UNUSED_VALUE)
CID 10929: Unused pointer value (UNUSED_VALUE)
CID 10928 (#1 of 1): Unused pointer value (UNUSED_VALUE)
returned_pointer: Pointer "p.h" returned by "addressof(h->handler_)" is never used. 
49    ptr p = { boost::addressof(h->handler_), h, h };
50
51    BOOST_ASIO_HANDLER_COMPLETION((h));
52
53    // Make a copy of the handler so that the memory can be deallocated before
54    // the upcall is made. Even if we're not about to make an upcall, a
55    // sub-object of the handler may be the true owner of the memory associated
56    // with the handler. Consequently, a local copy of the handler is required
57    // to ensure that any owning sub-object remains valid until after we have
58    // deallocated the memory here.
59    detail::binder1<Handler, boost::system::error_code>
60      handler(h->handler_, h->ec_);
61    p.h = boost::addressof(handler.handler_);

boost/asio/detail/wait_handler.cpp:

43  static void do_complete(io_service_impl* owner, operation* base,
44      const boost::system::error_code& /*ec*/,
45      std::size_t /*bytes_transferred*/)
46  {
47    // Take ownership of the handler object.
48    wait_handler* h(static_cast<wait_handler*>(base));

CID 11127 (2): Unused pointer value (UNUSED_VALUE)
CID 10928: Unused pointer value (UNUSED_VALUE)
CID 10929 (#1 of 1): Unused pointer value (UNUSED_VALUE)
returned_pointer: Pointer "p.h" returned by "addressof(h->handler_)" is never used. 
49    ptr p = { boost::addressof(h->handler_), h, h };
50
51    BOOST_ASIO_HANDLER_COMPLETION((h));
52
53    // Make a copy of the handler so that the memory can be deallocated before
54    // the upcall is made. Even if we're not about to make an upcall, a
55    // sub-object of the handler may be the true owner of the memory associated
56    // with the handler. Consequently, a local copy of the handler is required
57    // to ensure that any owning sub-object remains valid until after we have
58    // deallocated the memory here.
59    detail::binder1<Handler, boost::system::error_code>
60      handler(h->handler_, h->ec_);
61    p.h = boost::addressof(handler.handler_);

boost/asio/

Change History (5)

comment:1 by chris_kohlhoff, 9 years ago

Severity: ProblemCosmetic

I suspect the warning is spurious, as the member h is used by ptr's destructor. Will investigate later.

comment:2 by Richard <legalize@…>, 9 years ago

I looked at this pretty closely before opening the ticket. So far, Coverity has had a very low false positive on it's analysis because it analyzes the resulting binary as well as the source code.

To my eyes, the sequence goes like this in reactive_socket_recv_op.hpp:

ptr p = { boost::addressof(o->handler_), o, o };
// p.h == boost::addressof(o->handler_)

BOOST_ASIO_HANDLER_COMPLETION((o));
// p.h == boost::addressof(o->handler_)

detail::binder2<Handler, boost::system::error_code, std::size_t>
  handler(o->handler_, o->ec_, o->bytes_transferred_);
// p.h == boost::addressof(o->handler_)

p.h = boost::asio::detail::addressof(handler.handler_);
// p.h overwritten and original value discarded

p.reset();
// p.h used in d'tor if p.v != 0

All the other cases I looked at appeared to follow a similar pattern, but I only drilled through the exact details on this one.

comment:3 by chris_kohlhoff, 9 years ago

Doesn't seem to consider the possibility of exceptions.

comment:4 by Richard <legalize@…>, 9 years ago

If no exception is thrown, then the code executes as above.

comment:5 by Jeffrey Walton <noloader@…>, 9 years ago

Coverity offers its scanning service gratis to open and free projects. Boost can sign up for an account at http://scan.coverity.com/projects.

Note: See TracTickets for help on using tickets.