0

I've found some code like (many problems in the following code):

//setup consistent in each of the bad code examples
string someString; 
char* nullValue = getenv("NONEXISTENT"); // some non-existent environment variable  

// bad code example 1:
char x[1024];
sprintf(x," some text%s ", nullValue); //crashes on solaris, what about linux?  

// bad code example 2: 
someString += nullValue; // What happens here?  

//bad code example 3:
someString.append(nullValue); // What happens here?  

//bad code example 4:
string nextString=string(nullValue); //What happens here?
cout<<nextString;

We're using solaris, linux, gcc, sunstudio, and will quite possibly use clang++ in the future. Is the behaviour of this code consistent across platform and compiler? I couldn't find specs that describe expected behaviour in all the cases of the above code.

At present, we have problems running our code using gcc (and on linux), is the above code a likely cause?

If the code above acts the same in all of these environments, that's valuable information (even if the behavior is a crash) for me because I will know that this isn't the reason for our linux problems.

6
  • 3
    The sprintf call looks strange, shouldn't there be a %s somewhere ? Commented Jan 19, 2013 at 21:43
  • @J.N. You are correct, I fixed it. Commented Jan 19, 2013 at 21:46
  • @Dale: please post something realistic. x isn't used in that code, so the sprintf is pointless & confusing. And describe your actual problem. Commented Jan 19, 2013 at 21:48
  • @Mat That sprintf immediately crashes when run after compiling using sunstudio, that was actually the problem I am least interested in in the above code -- I expect it crashes everywhere (but couldn't find documentation saying that.) Commented Jan 19, 2013 at 21:52
  • If the sprintf is the problem, why incode all those strings around it in your sample? And why aren't you checking for nullValue being null before using it? Commented Jan 19, 2013 at 21:56

3 Answers 3

2

In general these uses of NULL, where a valid C string is expected, cause undefined behavior, which means that anything can happen. Some platforms try to have defined behavior for this. IIRC there are platforms that deal gracefully with passing a NULL pointer to printf family functions for a %s format substitution (printing something like "(null)"). Other than that, some platforms try to ensure a reproducible crash (e.g. a fatal signal) for such cases. But you can't rely on this in general.

If you have problems in that area of the code: yes, this is a likely cause or may obscure other causes, so: fix it, it's broken!

Sign up to request clarification or add additional context in comments.

1 Comment

I hate changing other people's code on this HUGE project. Especially when this problem is unrelated to what I'm supposed to be working on (and so were the last problems I fixed), so I was hoping to be able to ignore some of these things and work on what I'm really working on. The problem is if I work on all the problems in the code I see (this is only one of many), I won't get anything done but that and will get behind on my real tasks (which is why I'm here on a Saturday). Oh well, I guess today will be spent on other people's code (again).
1

There is a problem constructing strings from the pointer, without checking the return value first. The getenv definition says:

Retrieves a C string containing the value of the environment variable whose name is specified as argument. If the requested variable is not part of the environment list, the function returns a null pointer.

Creating a std::string from a null pointer is explicitly not allowed by C++ standard. The same goes for appending to the string (+=).

I'm no C expert, but have a hunch that using a null pointer with sprintf is not allowed either.

Comments

1

Exactly what happens when you use a NULL pointer in any of the cases you have described is "undefined behaviour". Some C libraries do recognise NULL for strings in printf, and will print "(null)" or something along those lines, but I would definitely not rely on that. Similarly, your other usages of NULL are "undefined", which means they are guaranteed to not work in any particular way across a range of platforms. What happens on one platform may well be completely different to what happens on another platform (or with another brand/version of the compiler, or with different compiler optimisation settings, or which way the wind blows that day if you are unlucky). In this case, it's likely that it leads to either a crash or "well behaved code" - it depends on who wrote the C/C++ library.

One solution, if you have a few of these things is to create a "getenv_safe" that instead of returning NULL returns an empty string [or "not set" or similar] if the environment variable isn't set, and then either fix the code directly, or #define getenv(x) getenv_safe(x).

5 Comments

I'll probably need to do this, dang it. #define won't work nicely for me because most of the code checks for null already and sometimes has other work to do if it is null.
Ok, then it's a case of "replace with new function where relevant". Do you use Doxygen or cscope to find your way around the code? They work quite well on identifying "where are all the calls to this function".
There are millions of lines of code, and thousands of calls to getenv -- I can find the bad uses easily enough (grep -A 10 ...), but just don't want to go through the whole process (we're CMMI 5) of getting an issue number, updating the code, getting everyone to review the changes, testing, etc. while I've got real work and deadlines for my own stuff that will be unaffected by my fixes to other people's code.
I feel your pain. Been there done that - you have to spend three times as much time doing "paperwork" than you do fixing the code, and it's genuinely needed to fix... Can't you raise one ticket for all the errors? "Fix many broken calls to getenv" or some such?
I can create one ticket, but I don't have permission to assign the issue to myself, if it's not assigned to me I can't create a branch, etc. etc. I just wanted to come in today and finish my issue and go home, not go through hours of paperwork and tedious fixes of code I'm not responsible for (the same kind of work I did last weekend). I'll just do it.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.