Thread: 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

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?

test code

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

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

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.

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

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...

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.

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");
    }
}

Originally Posted by ledow

It looks ugly

Yep, and I just made it even worse

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!