48

I'm reading the code of ROS.

In the file ros_comm/roscpp/include/ros/subscriber.h, I see such a piece of code:

operator void*() const { return (impl_ && impl_->isValid()) ? (void*)1 : (void*)0; }

Well, (void *)0 can be regarded as NULL in C, but what does (void *)1 mean?

If a class Foo contains this function, it means that we can code like this:

Foo foo;
void *ptr = foo;

Right? So does it mean that void *ptr = (void *)1 is possible? What does this mean?

Peter Mortensen
  • 30,738
  • 21
  • 105
  • 131
Yves
  • 11,597
  • 17
  • 83
  • 180
  • 9
    It casts 1 to a pointer and 0 to a pointer. That is all. – Brandon Jul 10 '19 at 02:25
  • 1
    That's a c-style cast. Thus, it's casting 1 to a `void *`. As to why you would want to do this, or what practical use this has, don't ask me. Ask the original programmer if you can, or maybe there's someone else here who can shed light on the subject. –  Jul 10 '19 at 02:27
  • It's probably used to locate what's in memory cell whose address is `1`. We don't know though, ask those who run the library. – lost_in_the_source Jul 10 '19 at 02:27
  • 13
    Possibly using a pointer as a bool implicit conversion. Not something you'd do that way in modern C++ (C++11 and later). – Eljay Jul 10 '19 at 02:33
  • I checked here: https://github.com/ros/ros_comm/blob/melodic-devel/clients/roscpp/include/ros/subscriber.h#L71 I don't see the cast used anywhere.. maybe it's used implicitly but that's a lot to search for so.. no idea.. – Brandon Jul 10 '19 at 02:50
  • @Brandon switch the branch to `kinetic-devel`, then you will find it – Yves Jul 10 '19 at 03:53

2 Answers2

74

This is an old trick to avoid problems with implicit conversions to bool from before explicit contextual conversions were introduced in C++11. It's intended to be used to check validity:

Subscriber my_subscriber = someFunction();
if (!my_subscriber) {
    // error case
}

The important point is that no built-in conversion exists from void* to integer types, but one does exist from bool to integer types. At the same time, a built-in conversion from void* to bool exists. That means that if you define an implicit conversion to bool, then the following is surprisingly valid:

void my_func(int i);

void another_func() {
    Subscriber sub = something();
    my_func(sub);
}

Defining a conversion to void* avoids that issue.


These days that trick is obsolete though. C++11 introduced explicit conversions. explicit conversions to bool are considered in the conditions of if and loops, but aren't considered in other problematic cases. That means that these days that conversion should be written as:

explicit operator bool() const { return impl_ && impl_->isValid(); }
Miles Budnek
  • 28,216
  • 2
  • 35
  • 52
  • Though the return-type used in the *safe bool Idiom* is normally a pointer to member-function so there is even less chance of misuse. – Deduplicator Jul 10 '19 at 15:49
-1

It shows that either the person who wrote the code isn't very well acquainted with the language or tools they're using, or the code has been around for a long, long time and been hacked on by different people, presumably having undergone a C-to-C++ transition at some time in the past, still carrying some legacy API contract (expecting a void*) which may be troublesome to change.

There is no good reason to do such a thing, if you look at the source. impl_ is a boost::shared_ptr<Impl> which implements operator bool, and Impl::isValid returns bool, too. There's no reason to use, or return anything but bool anywhere.

Basically, this is a contorted (and possibly dangerous) way of writing:

return impl_ && impl_->isValid();
Damon
  • 67,688
  • 20
  • 135
  • 185
  • Seems `boost::shared_ptr` [is a bit older than `explicit`](https://www.boost.org/doc/libs/1_70_0/libs/smart_ptr/doc/html/smart_ptr.html#history) and [historically used the safe bool idiom too](https://www.boost.org/doc/libs/1_31_0/libs/smart_ptr/shared_ptr.htm#conversions). – Deduplicator Jul 10 '19 at 18:24
  • 1
    @Deduplicator: It sure is older, but that's besides the point (the `explicit` keyword doesn't really make much of a difference here, none of the exposed API funcs makes use of it). The so-called safe bool idiom is a misnomer, though -- dirty hack would be better suited. It is by no means safe, or expresses intent well. A `bool` communicates: _"this is a yes or no value"_ whereas a `void*` communicates _"I am a pointer, please dereference me"_. Which, for `(void*)0` or `(void*)1`usually isn't such a brilliant idea. The fact that it "works" when incidentially used differently is irrelevant. – Damon Jul 11 '19 at 14:18
  • Programming is not just about _"works for me"_ or even _"works, I believe"_, but it is about making sure that things are well-defined, well-expressed, unambiguous, giving reproducable results and being as fail-safe as you can provide. Someone calling that function must immediately, without consulting 20 pages of documentation, know what to do with the return value. That's not the case with a `void*`. The notion _"yeah, but there's an implicit conversion to... whatever"_ isn't precisely good. – Damon Jul 11 '19 at 14:21
  • Nobody reasonable would ever deny that the safe bool idiom is a dirty hack, with all that implies. Unfortunately, it is also the best that could be done pre-C++11. And I only referenced `boost::shared_ptr`'s history because while you acknowledged that the code under discussion might be ancient, you curiously used the current incarnation of that library as an argument about the language version used. – Deduplicator Jul 11 '19 at 15:01
  • @Deduplicator: I'll have to admit that I'm not well acquainted with Boost versions of the mid-late-1990s :-) Though I'm pretty sure that `return !!m_ptr;` was well-defined and working perfectly reliably (with the exception of absence of an `explicit` keyword, obviously) in the late 80s already. Beats me why, for the sake of preventing a mostly harmless _possible and rarely occuring_ bool-to-int conversion, one would sacrifice clarity and expressivity. – Damon Jul 11 '19 at 15:48
  • Well, it was perfectly clear *in use*. The return-type is just a monstrosity to write, and sometimes leaked into diagnostics. Also, overload-resolution was AFAICT the key scenario. Well, let's consign it to the dustbin. – Deduplicator Jul 11 '19 at 15:55