• Advertisement
Sign in to follow this  

is "return((void*)1)" safe?

This topic is 3218 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

Hi, I have a function that returns a block of data depending on the arguments. Sometimes the functions doesn't return any data in which case the return value is set to NULL. However other times I just want to signal a special case without returning any data nor changing the number of arguments of my function. So would: return((void*)1); be safe? (knowing that the caller knows how to interpret this special case). Put in other words: return((void*)12345); is not safe because the same pointer value could be returned by a memory allocation function, but would the same memory allocation function ever return 1? Thanks!

Share this post


Link to post
Share on other sites
Advertisement
Perhaps someone will chime in with something more helpful, but if you are going to use a sentinel, -1 would be much safer. That maps into kernel reserved space and can't possibly be a legitimate memory address.

Share this post


Link to post
Share on other sites
It depends on your platform, but it's a pretty bad idea even if it was "safe".

Share this post


Link to post
Share on other sites
Quote:
Original post by Promit
That maps into kernel reserved space and can't possibly be a legitimate memory address.

Again, that depends on your platform. On a PC, yes. On an embedded platform, it may not be true.

Share this post


Link to post
Share on other sites
I've seen many libraries keep track of status/error codes from the last executed function.

Share this post


Link to post
Share on other sites
Quote:
Original post by SiCrane
Quote:
Original post by Promit
That maps into kernel reserved space and can't possibly be a legitimate memory address.

Again, that depends on your platform. On a PC, yes. On an embedded platform, it may not be true.
In my defense, I tend to assume people aren't on embedded devices, because those things usually throw half the rulebook away, and shred another quarter. If you have to preface everything with "but embedded will screw you over", it gets rather tedious.

Share this post


Link to post
Share on other sites
Could do something like this:

static int kDummy = 0;
void* kSpecialCase = &kDummy;

void* value = MyFunction();
if( value == NULL )
{
// error
}
else if( value == kSpecialCase )
{
// special case
}
else
{
// normal
}

Share this post


Link to post
Share on other sites
After reading this thread, I think that using dubious casts should be punishable offense.

So why not find a more reasonable solution?

Share this post


Link to post
Share on other sites
Quote:
Original post by Antheus
After reading this thread, I think that using dubious casts should be punishable offense.

In their defence, i think they said the code was generated be a decompiler, in which case the decompiler would not have any information on the class and method names.

ingramb's suggestion is the safest for returning a "special case" pointer, but i'm sure there are still better ways.

Share this post


Link to post
Share on other sites
It is a pretty common convention to return a null pointer to indicate that a function failed. I would argue that this would be more intuitive than using something like -1 since many people automatically expect this behavior and check for null pointers. Knowing that one needs to check for -1 would require one to be up to date on documentation, and although checking documentation is important, a very good codeset will be as intuitive as possible from it's interface alone.

Share this post


Link to post
Share on other sites
don't use some weird sentinel value. it's ugly and error prone.

Two possible solutions

1: Use an exception to indicate error (assuming this is C++ and not C).

2: Return an enum that clearly describes the state of the function and pass the data back as a either a result struct or a reference parameter i.e.

enum Result
{
Success,
Failure,
SpecialCase
}

// and either
struct ReturnValue
{
void* data;
Result result;
}
}

ReturnValue SomeFunc();

// or

Result SomeFunc(void** data)
{

}



neither of those are very nice, so the exception is probably the way to go if you can do it that way.

Share this post


Link to post
Share on other sites
Using something like Optional<T> would let you know the value is invalid, but null pointers handle that anyway. If you need to interpret the result, I'd find another way. Perhaps another function to call which can retrieve the error value, or pass in an additional parameter to store the error code if you really need it. If you hate all these ideas, now you know why exceptions exist.

Try to keep your code logical: null pointers means invalid, anything else is valid. If I saw your function and tried to use it, that's exactly what I'd expect to be returned.

Share this post


Link to post
Share on other sites
Quote:
Original post by ChaosEngine
1: Use an exception to indicate error (assuming this is C++ and not C).

I would advise against that. Exceptions are not meant for indicating common errors. When an exception is thrown, it involves flushing the CPU pipeline, alerting the operating system, unwinding the stack, and returning to user code. In other words, it's exceptionally slow (pun intended) compared to just setting a flag.

Exceptions are for things that really haven't been anticipated. Stuff that is reason for shutting down the entire application or at least a module. Especially in game development the performance hit for frequent exception throwing is unacceptable.
Quote:
2: Return an enum that clearly describes the state of the function and pass the data back as a either a result struct or a reference parameter i.e.

This is an excellent solution. And the syntax is really not that bad compared to try/catch blocks. Handling common errors simply isn't something you can tuck away so it has to be dealt with explicitely.

Anyway, it sounded to me like floatingwoods's function regularly hits the special case. Right? There might even be better solutions, like avoiding the special case early, or even avoiding it altogether, but then we need to know more about what exactly he's doing...

Anyway, there's another alternative: return a pointer to a static 'invalid' object that you can test against. This way you make sure that this pointer can never represent any other, valid object. But do document this really well so that people using this function know they should test for the invalid object before doing anything else.

Share this post


Link to post
Share on other sites
Quote:
Original post by C0D1F1ED
Anyway, there's another alternative: return a pointer to a static 'invalid' object that you can test against.

Isn't that essentially what ingramb suggested? Ah, but the object would have to be declared outside the function scope so calling code can access it. Yes, pretty messy.

Share this post


Link to post
Share on other sites
Quote:
Original post by floatingwoods
I have a function that returns a block of data depending on the arguments. Sometimes the functions doesn't return any data


You've already contradicted yourself. :)

Quote:
in which case the return value is set to NULL. However other times I just want to signal a special case without returning any data nor changing the number of arguments of my function.


What's special about this case, besides the fact that it doesn't return any data?

Share this post


Link to post
Share on other sites
Actually Zahlman you asked the right question.

My function is a function that is present in several extension modules (dlls) of my main application. The main application will broadcast a message to all modules. Each module can:

1) return NULL, in which case the broadcast continues
2) return data, in which case the broadcast is interrupted (other modules won't receive that message)
3) return "that special signal" indicating that the broadcast should be interrupted without having a return value

As many suggested, the problem can be solved in various ways without having the 3 distinct return values, but since I already have several extension modules, I was attempted to keep the old modules "as is", instead of rewriting the function (e.g. with different parameters). But I think being lazy never pays off in the long run...

Share this post


Link to post
Share on other sites
So if you do want to rewrite your old modules, you should use boost::optional<T>. Seems to be the right place to use it.

Share this post


Link to post
Share on other sites
Quote:
Original post by owl
I've seen many libraries keep track of status/error codes from the last executed function.


Which isn't all that nice either imho, as it either requires you to use thread local storage, or it wouldn't be possible to use the code from different threads at the same time, even when it's working on completely independent data.

Share this post


Link to post
Share on other sites
Quote:
Original post by floatingwoods
Actually Zahlman you asked the right question.
He tends to do that...[lol]

Share this post


Link to post
Share on other sites
Quote:
Original post by C0D1F1ED
Exceptions are for things that really haven't been anticipated. Stuff that is reason for shutting down the entire application or at least a module. Especially in game development the performance hit for frequent exception throwing is unacceptable.
I'm afraid neither of that is quite true. Exceptions are a means to recover from errors in a graceful manner without killing the process. Killing the process or shutting down a module is not error handling, it is failure.
If you use exceptions only to kill the process, you can as well use assert(), it has easier syntax, and less overhead (but again, this is failure, not error handling).

The performance hit for exceptions is not at all unacceptable, unless you throw maybe 20,000 times per second. While it is true that exceptions are relatively expensive, they also are exceptional, they don't occur that often.
Even a hundred exceptions per second are not noticeable at all, and your code likely won't throw that many (or you have other problems!).

Share this post


Link to post
Share on other sites
Quote:
Original post by SiS-Shadowman
So if you do want to rewrite your old modules, you should use boost::optional<T>. Seems to be the right place to use it.


QFE. I use it a lot. Together with tuple<> it helps me to write 99.9% of my functions in The Right Form "Function (input) -> output".

Share this post


Link to post
Share on other sites
boost::optional really isn't much of an improvement here. A pointer already has a value that indicates a missing value: null. Adding boost::optional to the mix means that you'll have two different representations for a missing value. This is hardly what you would call self-documenting code.

Share this post


Link to post
Share on other sites
Quote:
Original post by quasar3d
Quote:
Original post by owl
I've seen many libraries keep track of status/error codes from the last executed function.


Which isn't all that nice either imho, as it either requires you to use thread local storage, or it wouldn't be possible to use the code from different threads at the same time, even when it's working on completely independent data.


Well, at least it is cleaner than mixing return codes with unrelated return value types. Having it working in a per thread basis is the right way to do it thread safe. If you wanted to get different error codes from a function without changing it's interface, I really can't think of any other way to do it.

Share this post


Link to post
Share on other sites
The only reasonable numbers are zero, one, and infinity.

Your function returns two types of values: blocks of data and status.

Two is not a reasonable number.

You design is unreasonable.

You ask if it is reasonable to do something outside of accepted practise in order to salvage your unreasonable design. Unfortunately, unreasonability is commutative and distributive. The answer is that it is not reasonable to return (void*)1 in this case.

See, programming is a process of reasoning about the current and future state of a complex mathematical function. You cannot, by definition, reason about something unreasonable.

So, the solution is that you should change your design. If you would like a suggestion, return your data by parameter and your status by value (following the classic C idiom).

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement