returning an error code from a function that has a pointer return type

I have tried several google searches, but probably use the wrong keywords to find this.

I have a function that returns a pointer type, now if the function detects an error condition I would like the caller to be able to check the returned type for a specific error condition instead of only being able to return NULL as a failure indication.

I understand that pointers are numeric values int/long etc. but what's the best approach to do this and make sure I don't define a possible proper pointer value as an error code?

example of what I would like to achieve (not actual code used);

Code:

---

#define BADPARAMETER -1

void* myfunc(int myparam)
{
    if (myparam < 0) return BADPARAMETER;

    ....

}

---

caller code

Code:

---

result = myfunc(somevalue);
if (result == BADPARAMETER)
{
  .... handle returned error
}

---

Probably has been done before, so any pointers to a solution are appreciated.

Thijs

One approach:

Code:

---

void* myfunc(int myparam, int* err_code)
{
    if (myparam < 0)
    {
        if (err_code)
        {
            *err_code = BADPARAMETER;
        }
        return NULL;
    }

    /* .... */
}

---

So, if the caller wants more information, the caller provides the address of an int to which the error code is saved. Otherwise, the only indication of an error is the null pointer that is returned.

That's usually the best way.

The other method you often see is for the code to have a global error state stored somewhere. When the function errors, it returns NULL and changes the error state to some error code. This can then be queried (if the caller is interested in WHY the function failed) either by reading the global directly, or using an "accessor" function (e.g. GetLastError()) and return the last error code seen. Lots of large frameworks use something similar (which query functions called GetError or whatever).

The only problem with that is that you have to set the error code to something in every function that *could* return an error using it, even if that's to some "NO_ERROR" constant, so that ignored errors don't propagate for the entire life of the program. Oh, and the calling functions still have to check for "NULL" returns and do something about the error in their return-checking code.

The problem I find with the method described by laserlight is that often people who call your function "forget" to pass you something useful, and what if they pass you a NULL pointer to store the error in? Then the function has to check for NULL pointers and ignore errors as appropriate, so you end up with a lot of code "inside" the functions. Whereas with the "GetError" method, the function just sets a global int to an error value and returns NULL and it's up to the caller to do all the leg-work (if they care about the error at all).

Quote:

---

The only problem with that is that you have to set the error code to something in every function that *could* return an error using it, even if that's to some "NO_ERROR" constant, so that ignored errors don't propagate for the entire life of the program.

---

A lot of libraries don't go that route. Vendors will instead provide an interface to set the error code to a known "no error" value allowing client code to set the error code to that value if they care about handling a given error in advance.

Quote:

---

Then the function has to check for NULL pointers and ignore errors as appropriate, so you end up with a lot of code "inside" the functions.

---

I'm not one for accepting that "precondition" and "postcondition" checks should be a thing; I also don't think that is a valid argument against error code parameters.

Code:

---

unsigned int gErrorCode;

unsigned int GetErrorCode();

void SetErrorCode
(
    unsigned int fErrorCode
);

void * CraftSomething
(
    int fMyParameter
)
{
    if(fMyParameter < 0)
    {
        SetErrorCode(INVALIDPARAMETER);
        return(NULL);
    }
    else
    {
        // ...
    }
}

---

Code:

---

void * CraftSomething
(
    int fMyParameter
  , unsigned int * fErrorCode
)
{
    unsigned int lErrorCode = 0;
    if(!fErrorCode)
    {
        fErrorCode = &lErrorCode;
    }
    if(fMyParameter < 0)
    {
        fErrorCode = INVALIDPARAMETER;
        return(NULL);
    }
    else
    {
        // ...
    }
}

---

The implementations of real functionality isn't that different.

[Edit]
You should make a far reaching design decision like this based on how it affects client code.

Yea, I'm not sure how that "not" got there...
[/Edit]

Soma

Thanks for the quick and thorough responses. So my googling didn't turn anything up because it can't be done that way.
Disappointing, but not giving up, I created the test code below;

Code:

---

static char _____Errors[2];
void* MYLIB_NOMEMORY = &_____Errors[0];
void* MYLIB_BADPARAMETER = &_____Errors[1];
void* MYLIB_SOMEERROR = &_____Errors[2];


char* myfunc(int someparam)  // use char to test compiler type checking
{
    if (someparam < 0) return (char *)MYLIB_BADPARAMETER;
    if (someparam == 0) return (char *)MYLIB_SOMEERROR;
    return NULL;
}


void checkvalue(int param)
{
    void* ptr = myfunc(param);
    if (ptr == MYLIB_BADPARAMETER)
    {
        printf("Bad parameter\n");
    }
    else
    {
        if (ptr == MYLIB_SOMEERROR)
        {
            printf("Some error\n");
        }
        else
        {
            printf("nice, finally a proper value!\n");
        }
    }
}


int _tmain(int argc, _TCHAR* argv[])
{
    checkvalue(-2);
    checkvalue(-1);
    checkvalue(0);
    checkvalue(1);
    checkvalue(2);
    return 0;
}

---

It works, just as the regular macros work except that it uses variables in this case. Writing and reading source code are just as usual, only caveat is the explicit cast to the char* return type of the errors, but that will be verified at compile time.

So if I (c newbie) can come up with this, and you didn't (c experts), than I am probably missing some serious issues with this one.

So whats wrong with it?

Quote:

---

test code

---

I hate you and hope you suffer a minor torment... like a bad hangnail. I hope you get a bad hangnail.

Quote:

---

So whats wrong with it?

---

O_o

Let's ignore all the important stuff and focus on the trivial which I can't help but think will speak to a newbie more than the important stuff possibly could.

So, didn't you notice that you wrote a function that exists only to call your function which does not actually report an error to the level at which that function was called?

You didn't write the `checkvalue' for the testbed so much as you wrote it to reduce complexity of calling the real `myfunc' function. (We all know you did so can we please skip the "knee jerk" reaction where you say you didn't?) That, if nothing else, is a huge issue.

Instead of actually calling `myfunc' and checking one result that says "Successful!" you have devised a way that requires client code to check every possible error condition. That is nothing short of insane, but I do admit, it is one hell of an accomplishment to shock me.

To be perfectly clear, you have wrapped `myfunc' with `checkvalue' precisely because you have broken the clear and obvious logic of returning an error code. Here you are only checking for two conditions because you know in advance that the target function only supports two error conditions. In the real world, you would have to check every possible condition at every invocation only you can't because new conditions can be added at any time. Even if you devise a way to do this "magically" (I also hate anyone who helps you in that project.) you still have to conceptually check every case every invocation which will drag performance down at an alarming rate.

In other words, the "nice, finally a proper value!\n" isn't valid. You've only eliminated two of infinitely many (well 2^32-1 anyway) cases. You can't know at this point in time that you will not add another error case to `myfunc' later.

The thing is, you actually already taken the time to wrap the actual function; you've already realized that you will never issue those checks every time you want to invoke the function. You've already found and promptly eliminated this insanity by wrapping it up so you don't have to deal with it.

Actually, I can't believe how broke this is; the longer I think about it the more ways I think of to break it.

I hate you!

Soma

Ick.

It looks ugly, it introduces unnecessary globals, and you have to do handfuls of comparisons for every single function return. That's fine for a tiny program, but as you scale up that will become a huge performance drag (and sod having to code the checkvalue routine and maintain it every time there's a new type of error you want to catch).

You could have just let checkvalue do the same as laserlight suggested - little to no performance hit, easier syntax, much more maintainable and much more expandable when your code changes. Maybe for your case it "works" but it's hardly pretty and now do a good programming exercise: Imagine that someone else has to come along and use your "myfunc" function without your assistance from *their* code. They now would have to duplicate ALL your error handling code and your horrible static char array in order to use the damn thing.

This makes me physically ill.

I feel like I'm watching a horror movie where the bad guy stops in the middle of slaughtering an innocent turning to face the camera and actually saying "I'm Bob and I'll be your bad guy for this movie.". It isn't funny. It isn't clever. It is sick, sad, and pathetic!

Soma

Isn't using errno a simpler solution ?
It is global state but I don't think that is much of a problem in a procedural language, when guaranteed to be thread safe by the standards.

Quote:

---

Originally Posted by manasij7479

Isn't using errno a simpler solution ?
It is global state but I don't think that is much of a problem in a procedural language, when guaranteed to be thread safe by the standards.

---

There is nothing in the standards that guarantees usage of errno to be "thread safe".

Generally, when using functions that set errno, it is necessary to either (1) ensure they will never touch errno (i.e. code to avoid error states that needs to be tested for) or (2) synchronise access to the functions, so that access of errno is synchronised when access is attempted from multiple threads.

It is a basic fact of multithreaded development that no object (global or otherwise) can protect itself from concurrent access. It is always up to code which uses that object (eg the callers of functions that modify errno) to handle synchronisation, not the objects themselves.

O_o

The standard I know (C99) doesn't guarantee thread safety, but I do think it references atomicity in the latest standard (C11). That though only guarantees atomic reads and writes meaning that the issues grumpy references still come into play.

That said, some implementations bypass the concurrency issue by using thread local storage (using compiler intrinsic). You can't rely on this.

Soma

Quote:

---

Originally Posted by grumpy

It is a basic fact of multithreaded development that no object (global or otherwise) can protect itself from concurrent access.

---

But errno may not be an object...but just an obfuscation.. afaik. (It can link to a function call like mechanism, which implements the locking code, unless it is local to threads)
My man page says:

Quote:

---

errno is defined by the ISO C standard to be a modifiable lvalue of type int, and must not be explicitly
declared; errno may be a macro. errno is thread-local; setting it in one thread does not affect its value in
any other thread.

---

Quote:

---

Originally Posted by phantomotap

That said, some implementations bypass the concurrency issue by using thread local storage (using compiler intrinsic). You can't rely on this.

---

What is the difference ?
I thought they were exactly the same as far as the code is concerned.

It doesn't matter what `errno' really is if it is shared between threads because of how it is used.

I'll leave these examples here for you to examine.

Code:

---

// This is not sufficient to protect `errno' use
// patterns because atomicity only guarantees
// reads and writes will be complete within the
// context of one thread of execution.
__attribute__((__atomic__)) int errno;

bool DoSomething1()
{
    // ...
    if(/* ??? */)
    {
        // This is guaranteed to set `errno' to the value 1
        // completely and truly even if another thread
        // tries to set a different value at the same time.
        errno = 1;
        return(false);
    }
    // ...
}

void DoSomething2()
{
    // ...
    errno = 0;
    if(!DoSomething1())
    {
        // The `errno' is only atomic with respect to reads and
        // writes. The value `errno' has is going to be the
        // complete and true value any thread set.
        // However, it is not guaranteed to be the value
        // `DoSomething1' set because another thread may
        // have changed the value after `DoSomething1' set the
        // value but before we were able to examine it here.
        printf("%d", errno);
    }
    // ...
}

---

Code:

---

__attribute__((__tls__)) int errno;

bool DoSomething1()
{
    // ...
    if(/* ??? */)
    {
        errno = 1;
        return(false);
    }
    // ...
}

void DoSomething2()
{
    // ...
    errno = 0;
    if(!DoSomething1())
    {
        // Because we have implemented `errno' in terms of
        // thread local storage no other threads can
        // have modified `errno'. We have the exact `errno'
        // value `DoSomething1' set.
        printf("%d", errno);
    }
    // ...
}

---

Soma

So... I crafted evil code and will burn in hell forever...

Quote:

---

Originally Posted by phantomotap

Instead of actually calling `myfunc' and checking one result that says "Successful!" you have devised a way that requires client code to check every possible error condition. That is nothing short of insane, but I do admit, it is one hell of an accomplishment to shock me.

---

So here's my big miss. Thx for pointing that out, that is indeed a big issue.

Quote:

---

Originally Posted by phantomotap

In the real world, you would have to check every possible condition at every invocation only you can't because new conditions can be added at any time. Even if you devise a way to do this "magically" (I also hate anyone who helps you in that project.) you still have to conceptually check every case every invocation which will drag performance down at an alarming rate.

In other words, the "nice, finally a proper value!\n" isn't valid. You've only eliminated two of infinitely many (well 2^32-1 anyway) cases. You can't know at this point in time that you will not add another error case to `myfunc' later.

---

Those arguments can be overcome, the array has consecutive elements, hence pointer arithmetic can be used ;

Code:

---

#define MYLIB_MAXERROR 30
static char _____Errors[MYLIB_MAXERROR];
void* MYLIB_NOMEMORY = &_____Errors[0];
void* MYLIB_BADPARAMETER = &_____Errors[1];
void* MYLIB_SOMEERROR = &_____Errors[2];
#define MYLIB_ISERROR(ptr) (ptr >= &_____Errors[0] && ptr <= &_____Errors[MYLIB_MAXERROR])

---

with usage;

Code:

---

void dosomethingthatmightcauseanerror(int param)
{
    void* ptr = myfunc(param);
    if (MYLIB_ISERROR(ptr))
    {
        printf("We had an error\n");
    }
    else
    {
        printf("A decent value\n");
    }
}

---

Quote:

---

Originally Posted by ledow

It looks ugly

---

Yep, and I just made it even worse

Quote:

---

Originally Posted by ledow

it introduces unnecessary globals

---

Is that such a bad thing, compared to lists of integer macros to define errors, which seems to be quite common in c? Seems the same to me.


But as said, I think laserlights suggestion would be the cleanest solution. I think I'll go for that.

Thanks for your time folks!

Quote:

---

Originally Posted by tieske

Those arguments can be overcome, the array has consecutive elements, hence pointer arithmetic can be used ;

---

I believe that using <= or >= to compare pointers that do not point to elements of the same array (or one past the end thereof; or to the same object) results in undefined behaviour.

O_o

That only overcomes one of those arguments and only by changing the rules.

If you were to add "report a specific error condition" back you would still need a linear scan.

Of course, you can fix that. Of course, I can tell you how that fix is also broken. We could play that game forever. That's why this is so sickening. It is broken in such a profound way that every attempt to fix it will only change the flavor of its "brokeness".

This concept of yours is extremely brittle and broken beyond belief because everything you try to fix it just paints the curtains when the foundation is flawed.

Again, I actually congratulate you on the concept. The concept is broken beyond belief. Willingness to pursue an insane concept is solid win.

Soma

Quote:

---

Originally Posted by laserlight

I believe that using <= or >= to compare pointers that do not point to elements of the same array (or one past the end thereof; or to the same object) results in undefined behaviour.

---

That is wat the example does? or am I missing something. As long as all the errorcodes are defined using the same array, it works.

Quote:

---

Originally Posted by tieske

That is wat the example does? or am I missing something. As long as all the errorcodes are defined using the same array, it works.

---

Well, in that case, why return a pointer? You might as well return an int, since you are not returning a pointer to an object that does not represent an error code, so this whole business about pointers is much ado about nothing. But if you return a pointer to an object that does not represent an error code, then the object may well not be part of this error code array.

Quote:

---

Originally Posted by phantomotap

If you were to add "report a specific error condition" back you would still need a linear scan.

---

It does, it has a single check to see whether there is an error, and only in the error case you would traverse the possible error codes to find out what exactly went wrong. This is no different than reporting some integer, or even using errno.

Quote:

---

Originally Posted by phantomotap

That's why this is so sickening. It is broken in such a profound way that every attempt to fix it will only change the flavor of its "brokeness".

This concept of yours is extremely brittle and broken beyond belief because everything you try to fix it just paints the curtains when the foundation is flawed.

Again, I actually congratulate you on the concept. The concept is broken beyond belief. Willingness to pursue an insane concept is solid win.

---

Could you please provide the argument instead of just ranting? I'm trying to learn why it is brittle and broken, and I do appreciate the input.

Quote:

---

Could you please provide the argument instead of just ranting?

---

Yea, the fact that you think I'm just ranting lost you my interest.

I may have been ranting, but my every criticism is real.

Maybe instead of trying to rescue the concept with attempts to correct the code and waving my comments away with "He is a lunatic." you should take the time to reason about what I've said along with your code. While I am a lunatic, I am also very good at what I do.

Soma

Quote:

---

Originally Posted by laserlight

Well, in that case, why return a pointer? You might as well return an int, since you are not returning a pointer to an object that does not represent an error code, so this whole business about pointers is much ado about nothing. But if you return a pointer to an object that does not represent an error code, then the object may well not be part of this error code array.

---

A now I get your comment. Maybe I should have been clearer in my initial post. It is very common with functions that return an integer, that an impossible range of outcomes is used as error signals. Consider a 'length' function that determines the length of some object or whatever and returns an int. As length should always be positive (lets assume that for this example), negative values may be returned to signal a problem with the object or input parameters. This is quite common practice.

My intent was to get a similar behaviour, but in this case with a function that returns a pointer, instead of an int. So the example only shows the error returned, but obviously in non-example code, if there wouldn't be an error, it should return a pointer to a struct, string, object, whatever the function was to return on its 'good-path'.
So only have a single return value, which encodes both proper return values (good-path) and multiple possible error codes (failure-path) instead of only NULL for an error.

To be able to do this, the error code pointer may not ever be equal to a possible valid return value. Hence the consecutive array, any pointer within this memory area is an error, any outside is a valid value (presumed the function did what it was supposed to do). The macro

MYLIB_ISERROR just checks whether the pointer is within the error area (= the array)

I hope it makes more sense now.

You're not telling me anything new, and my point in post #16 still stands.

Quote:

---

Originally Posted by laserlight

You're not telling me anything new, and my point in post #16 still stands.

---

I (wrongly) assumed pointers to be mere integer values denoting a memory address from 0 to end-of-memory, in byte order. How wrong I am; K&R, 2nd edition, Ch 5, p. 102;

Quote:

---

Originally Posted by Kernighan and Ritchie

"Pointers may be compared under certain circumstances. If p and q point to members of the same array, then relations like ==, !=, <, >=, etc., work properly. For example,
p < q
is true if p points to an earlier member of the array than q does. Any pointer can be meaningfully compared for equality or inequality with zero. But the behavior is undefined for arithmetic or comparisons with pointers that do not point to members of the same array. (There is one exception: the adress of the first element past the end of an array can be used in pointer arithmetic.)

---

I would still expect it to work, but it can't be relied upon. So clear point why it can't be used.

Finally got the (rather important and hence valuable) message, thanks for teaching and for your patience!

Quote:

---

Originally Posted by manasij7479

But errno may not be an object...but just an obfuscation..

---

I was using "object" in a very generic sense: any entity with state or behaviour.

Quote:

---

Originally Posted by manasij7479

afaik. (It can link to a function call like mechanism, which implements the locking code, unless it is local to threads)

---

Even if it was a function call, a function cannot protect itself from being called by multiple threads. As I said, I was using the term "object" very generically (not, for example, in a way that a C++ programmer might - this thread is in a C forum, after all).

Quote:

---

Originally Posted by manasij7479

My man page says:

---

The 1999 C standard only says this, starting partway through Section 7.5 para 2.

Code:

---

and
      errno
which expands to a modifiable lvalue  that has type int, the value of which is set to a positive error number by several library functions. It is unspecified whether errno is a
macro or an identifier declared with external linkage. If a macro definition is suppressed in order to access an actual object, or a program defines an identifier with the name
errno, the behavior is undefined.

---

There is no reference to errno being thread-local. Then again, C99 states no requirements related to threads (it assumes a single thread of execution) nor to thread local storage. Given a disagreement between a man page and a standard, I'll argue the standard is correct.

Multithreading support is actually optional in C11. For that reason, even if you have a compiler that supports C11, it is a fair bet that there would be no requirement that errno be in thread-local storage. That doesn't mean errno cannot be in TLS: it means that your man page is specific to a particular platform.