strings and malloc()

[Mark]

Hello

Given this little piece of code.

#include <stdio.h>
static char *mkw(void)
{
return "xyz";
}

int main(void)
{
char *s = NULL;
s = mkw();
printf("%s\n", s);
return 0;
}

Running 'splint' ("splint foo.c") results in complaints:

Splint 3.1.1 --- 11 Sep 2006

a.c: (in function mkw)
a.c:5:9: Observer storage returned without qualification: "xyz"
Observer storage is transferred to a non-observer reference. (Use
-observertrans to inhibit warning)
a.c:5:9: Storage becomes observer
a.c: (in function main)
a.c:14:11: Fresh storage s not released before return
A memory leak has been detected. Storage allocated locally is not released
before the last reference to it is lost. (Use -mustfreefresh to inhibit
warning)
a.c:11:2: Fresh storage s created

Finished checking --- 2 code warningsSplint 3.1.1 --- 11 Sep 2006

What's wrong with it? GCC happily compiles though. I read that sometimes
'splint' may be too picky, is this the case?
Thanks in advance.

 

[Paul Hsieh]

Hello

Given this little piece of code.

#include <stdio.h>
static char *mkw(void)
{
   return "xyz";
}

int main(void)
{
   char *s = NULL;
   s = mkw();
   printf("%s\n", s);
   return 0;
}

Running 'splint' ("splint foo.c") results in complaints:

Splint 3.1.1 --- 11 Sep 2006

a.c: (in function mkw)
a.c:5:9: Observer storage returned without qualification: "xyz"
Observer storage is transferred to a non-observer reference. (Use
-observertrans to inhibit warning)
a.c:5:9: Storage becomes observer
a.c: (in function main)
a.c:14:11: Fresh storage s not released before return
A memory leak has been detected. Storage allocated locally is not released
before the last reference to it is lost. (Use -mustfreefresh to inhibit
warning)
a.c:11:2: Fresh storage s created

Finished checking --- 2 code warningsSplint 3.1.1 --- 11 Sep 2006

What's wrong with it? GCC happily compiles though. I read that sometimes
'splint' may be too picky, is this the case?
Thanks in advance.

Splint's problem is that you are returning a char *, which the
receiver can happily attempt to modify. However, when you execute a
return "xyz"; you are returning a const char * which is supposed to be
unmodifiable. Basically SPLINT wants you to either declare the thing
as const or copy the output to some modifiable storage location (a
static buffer or an allocated buffer; *NOT* something from the stack,
which is much worse, but which SPLINT will happily tell you about
anyway.)

The reason GCC compiles it is that its legal C according to the
standard.

You made the function static, which means in theory SPLINT can check
all the call-sites to know that you aren't modifying the return.
However, since you pass it to printf() which is implemented with a
vararg parameterization, it means it cannot determine whether or not
it will be written to.

 

[Ben Bacarisse]

Given this little piece of code.

#include <stdio.h>
static char *mkw(void)
{
return "xyz";
}

int main(void)
{
char *s = NULL;
s = mkw();
printf("%s\n", s);
return 0;
}

Running 'splint' ("splint foo.c") results in complaints:

Splint 3.1.1 --- 11 Sep 2006

a.c: (in function mkw)
a.c:5:9: Observer storage returned without qualification: "xyz"
Observer storage is transferred to a non-observer reference. (Use
-observertrans to inhibit warning)
a.c:5:9: Storage becomes observer
a.c: (in function main)
a.c:14:11: Fresh storage s not released before return
A memory leak has been detected. Storage allocated locally is not released
before the last reference to it is lost. (Use -mustfreefresh to inhibit
warning)
a.c:11:2: Fresh storage s created

Finished checking --- 2 code warningsSplint 3.1.1 --- 11 Sep 2006

What's wrong with it? GCC happily compiles though. I read that sometimes
'splint' may be too picky, is this the case?

Splint has a model of abstract types and can use this to warn about
places where information about representations leaks out of function.
It uses that same data flow analysis to detect modifications of string
literals, so the warning above is really an accident ("I treat strings
as abstract and you are returning a pointer into one without
permission").

Writing

static /*@observer@*/ char *mkw(void)

silences all these warnings by goving the function permission to retrn
observer storage. In addition, splint can now warn you if you attempt
to modify the string.

Is this splint "getting it wrong"? It depends what you want from your
tools. Splint is very cautious and to get code to pass though
silently often needs lots of annotations but they almost always get
you some benefit (like the detection of modification above) so you
have to decide if the effort is worth it.

 

[Ben Bacarisse]

Splint's problem is that you are returning a char *, which the
receiver can happily attempt to modify. However, when you execute a
return "xyz"; you are returning a const char * which is supposed to be
unmodifiable. Basically SPLINT wants you to either declare the thing
as const or copy the output to some modifiable storage location (a
static buffer or an allocated buffer; *NOT* something from the stack,
which is much worse, but which SPLINT will happily tell you about
anyway.)

The reason GCC compiles it is that its legal C according to the
standard.

You made the function static, which means in theory SPLINT can check
all the call-sites to know that you aren't modifying the return.
However, since you pass it to printf() which is implemented with a
vararg parameterization, it means it cannot determine whether or not
it will be written to.

I don't think this really covers it. If we solve both issues (printf
and the non-const char) splint still complains. This:

#include <stdio.h>
static const char *mkw(void) { return "xyz"; }

int main(void)
{
const char *s = NULL;
s = mkw();
printf("%c\n", s[0]);
return 0;
}

gives me the same warnings from splint as the original.

 

[Phil Carmody]

Piggybacking, sorry.


Nope, you're returning a char*. You're thinking of C++, perhaps?

Phil

 

[Keith Thompson]

When you execute a return "xyz", you are returning a (char *).

That's true, of course. But a function like this:

static char *mkw(void)
{
return "xyz";
}

is potentially dangerous, because it pretends to return a pointer to
*modifiable* char. Someone writing a call to mkw who believes its
promise might write something like:

char *result = mkw();
*result = 'X';

and it's not unreasonable for a tool to complain about it. By changing
the function to

static const char *mkw(void)
{
return "xyz";
}

you (a) implicitly convert the char* result of the implicit conversion
of "xyz" from char[4] to char* to const char*, and thereby (b) make it
more difficult for the caller to attempt to clobber the (array created
by the) string literal.

I'm having a little trouble myself parsing what I just wrote, so I'll
break it down. "xyz" is of type char[4], with the value { 'x', 'y',
'z', '\0' }. Since it's not the operand of a unary "&" or sizeof, or
used to initialize an array, it's converted from char[4] to char*.
The type char* implies that you can modify what it points to, but you
can't (it's undefined behavior); this is arguably a flaw in the
language, but one that was necessary for backward compatibility. The
return statement implicitly converts the result from char* to const
char*; the actual type of the expression is char*, but const char*
more accurately (and safely) indicates what it really is: a pointer to
unmodifiable char.

 

[Guest]

Piggybacking, sorry.



Nope, you're returning a char*. You're thinking of C++, perhaps?

what?

 

[jameskuyper]

what?

In C, "xyz" has the type char[4], which decays to char* in most
contexts. In C++, "xyz" has the type "const char[4]", which decays to
const char*. Paul Hseih's statement makes more sense if he was
thinking of C++ rules than it does if he's using C rules. In neither
language is it safe to assume that the memory set aside to hold that
string is writable, but that's a separate issue from the type
qualifiers.

 

[CBFalconer]

You have posted a correct program.

There is no memory leak.
True.

The memory for the string is reserved with static duration
and initialized before main starts.

False. The constant char string "xyz" is accessed, and its address
is returned, in function mkw AFTER the program begins to run. That
string is unmodifiable, but is perfectly printable. The word
'static' in the function definition refers to the function, not to
the type of memory used for the returned value, and makes the
function strictly local to the file.

 

[Keith Thompson]

Given this little piece of code.

#include <stdio.h>
static char *mkw(void) {
return "xyz";
}

int main(void) {
char *s = NULL;
s = mkw();
printf("%s\n", s);
return 0;
}

[...]
You have posted a correct program.

There is no memory leak.
True.

The memory for the string is reserved with static duration
and initialized before main starts.

False.

No, it's true.

The constant char string "xyz" is accessed, and its address
is returned, in function mkw AFTER the program begins to run.

Yes, but the array that holds the value "xyz" has static storage
duration and is created and initialized before main starts. (An
implementation can do this later if the program can't tell the
difference.)

That
string is unmodifiable, but is perfectly printable.

Certainly. (It's unmodifiable in the sense that attempting to modify
it invokes undefined behavior.) Do you think this contradicts what
pete wrote?

The word
'static' in the function definition refers to the function, not to
the type of memory used for the returned value, and makes the
function strictly local to the file.

Correct, but irrelevant. Storage for string literals has static
storage duration, and is initialized during program startup; the
"static" keyword on the function definition doesn't affect this one
way or the other.

See C99 6.4.5p5.