Is documentation for reallocate consistent?

[jacquelinekay]

https://github.com/ros2/rcl/blob/master/rcl/include/rcl/allocator.h#L45-L52

The comment for "reallocate" implies to me that if the field is left null, then a reallocate function will be constructed from "deallocate" and "allocate". I wasn't sure where in the implementation this actually happens though; it seems as if realloc will simply throw an error in this case.

https://github.com/ros2/rcl/blob/master/rcl/src/rcl/allocator.c#L54-L64

There are also several places were an error is returned in reallocate is null, e.g. https://github.com/ros2/rcl/blob/master/rcl/src/rcl/wait.c#L126-L127

It's not an issue in terms of what's needed in the implementation, but we should take a moment to make sure implementation and documentation are consistent.

[wjwwood]

https://github.com/ros2/rcl/blob/master/rcl/include/rcl/allocator.h#L45-L52

This linked comment doesn't mention that realloc can be a null function. I think the meaning (of this comment) is that any function pointed to from here should do the realloc behavior if possible, but if not it should alloc and then dealloc if successful.

There are also several places were an error is returned in reallocate is null, e.g. https://github.com/ros2/rcl/blob/master/rcl/src/rcl/wait.c#L126-L127

That needs realloc to be set, because it just wraps it and deallocates the input pointer even if the alloc fails, i.e. the behavior of reallocf.

So I don't think it's out of step, but maybe I'm reading it differently than you. Is there a clearer statement which indicates that the realloc pointer should not be null?

[jacquelinekay]

Maybe it was just wishful thinking on my part.

As a side note, since realloc is not provided in std::allocator_traits, I created realloc from dealloc and alloc in rclcpp. Unfortunately my approach didn't work on Windows (caused a segfault in every rclcpp test). I've thrown up my hands in despair and ifdef'd out custom allocators in Windows so that the use_rcl branch can get merged:

ros2/rclcpp@c1fc16e#diff-43afa33f042770415fd58f1b74c20fcdR72

But my next step will probably be to play around with realloc on Windows to see if we can make it work and if API changes will need to be made to rcl.

[wjwwood]

Ok, well unless you think we should change it, can you close the issue?