Can this cause a program to crash?

[weidongtom]

Hi,

I was reading some code and I came across this function:


static char *
base_name(char *s)


{

char *bp;

char *ep;


bp = s;

ep = 0; /* Can this cause problem?*/

while (*s)

{

if (s[0] == '/' && s[1] && s[1] != '/')

bp = s + 1;

if (s > bp && s[0] == '/' && s[-1] != '/')

ep = s;

++s;

}

if (!ep)

ep = s;

*s = 0;

return bp;

}

ep = 0; Memory is not allocated to ep, so, this could write to any
memory address right? And I tried it out with:

#include <stdio.h>

int main(void){
char *b;
*b = 0;
return 0;
}

and I get a segmentation fault. So I guess that's a bug right? (This
is from the source code of hexdump-1.5).

 

[Richard Bos]

static char * base_name(char *s)
{
char *bp;
char *ep;

bp = s;
ep = 0; /* Can this cause problem?*/
No.

while (*s)
{
if (s[0] == '/' && s[1] && s[1] != '/')
bp = s + 1;
if (s > bp && s[0] == '/' && s[-1] != '/')
ep = s;
++s;
}
if (!ep)
ep = s;

However, you never actually use ep for anything, so there's probably a
bug in your algorithm. At a random first guess, here:

*s = 0;

Also beware of passing string literals to this function. You're writing
to its argument, and you're not supposed to write to string literals.

ep = 0; Memory is not allocated to ep, so, this could write to any
memory address right?

Wrong. *ep is not ep. You're writing a null pointer value to ep, which
is legal. Writing to *ep would be undefined behaviour, but you're not
doing that.

And I tried it out with:

#include <stdio.h>

int main(void){
char *b;
*b = 0;

Here, you do write to *b, and not to b. So yes, this one is indeed
undefined behaviour, and could easily segfault or trample over the wrong
data.

Richard

 

[Richard Heathfield]

(e-mail address removed) said:

I was reading some code and I came across this function:

static char *base_name(char *s)
{
char *bp;
char *ep;
bp = s;
ep = 0; /* Can this cause problem?*/

No. You are simply assigning a new value to ep. It was indeterminate,
and now it is merely invalid - a significant improvement, since you can
test for invalidity, whereas you cannot test for indeterminism.

ep = 0; Memory is not allocated to ep, so, this could write to any
memory address right?

You are confusing ep = 0; with *ep = 0;

And I tried it out with:

#include <stdio.h>

int main(void){
char *b;
*b = 0;

Not the same thing at all. The * makes all the difference.

 

[Vinoj]

Hi,

I was reading some code and I came across this function:

static char *
base_name(char *s)

{

char *bp;

char *ep;

bp = s;

ep = 0; /* Can this cause problem?*/

while (*s)

{

if (s[0] == '/' && s[1] && s[1] != '/')

bp = s + 1;

if (s > bp && s[0] == '/' && s[-1] != '/')

ep = s;

++s;

}

if (!ep)

ep = s;

*s = 0;

return bp;

}

ep = 0; Memory is not allocated to ep, so, this could write to any
memory address right? And I tried it out with:

#include <stdio.h>

int main(void){
char *b;
*b = 0;
return 0;

}

and I get a segmentation fault. So I guess that's a bug right? (This
is from the source code of hexdump-1.5).

The function is for parsing the last '/' in a web address or a network
path (in unix) returns the pointer to the first character of the
filename of the web address. I think it has some redundant
informations.

Thanks, Regards,
Vinoj

 

[Flash Gordon]

CBFalconer wrote, On 09/05/07 15:16:

I was reading some code and I came across this function:

function reformatted to be visible in one page. Why double
linefeeds?

static char *
base_name(char *s) {
char *bp;
char *ep;

bp = s;
ep = 0; /* Can this cause problem? */ /* NO. NULL better */
while (*s) {
if (s[0] == '/' && s[1] && s[1] != '/') bp = s + 1;
if (s > bp && s[0] == '/' && s[-1] != '/') ep = s;
++s;
}
if (!ep) ep = s;
*s = 0;
return bp;
}

Seems valid. Convoluted, but valid. Mishandles "\n".

<snip>

It is probably not intended to handle \n or non-printable characters,
but only sane file names. This assessment is based on some off-topic
knowledge.

 

[Keith Thompson]

CBFalconer wrote, On 09/05/07 15:16:

I was reading some code and I came across this function:

function reformatted to be visible in one page. Why double
linefeeds?

static char *
base_name(char *s) {
char *bp;
char *ep;

bp = s;
ep = 0; /* Can this cause problem? */ /* NO. NULL better */
while (*s) {
if (s[0] == '/' && s[1] && s[1] != '/') bp = s + 1;
if (s > bp && s[0] == '/' && s[-1] != '/') ep = s;
++s;
}
if (!ep) ep = s;
*s = 0;
return bp;
}

Seems valid. Convoluted, but valid. Mishandles "\n".

<snip>

It is probably not intended to handle \n or non-printable characters,
but only sane file names. This assessment is based on some off-topic
knowledge.

The function could be simplified considerably by using the strrchr()
function.

I'm not sure what Chuck means when he says that it mishandles "\n".
As far as I can tell, it simply treats it like any other character;
only '/' and '\0' are treated specially. Based on the same off-topic
knowledge that Flash used, that's exactly the right thing to do.

<OT>On Unix-like systems, '\n' is a valid character in a file name;
only '/' and '\0' are disallowed, though other funny characters can
cause problems for some applications.</OT>

 

[Flash Gordon]

Keith Thompson wrote, On 09/05/07 21:50:

CBFalconer wrote, On 09/05/07 15:16:

:
I was reading some code and I came across this function:
function reformatted to be visible in one page. Why double
linefeeds?

static char *
base_name(char *s) {
char *bp;
char *ep;

bp = s;
ep = 0; /* Can this cause problem? */ /* NO. NULL better */
while (*s) {
if (s[0] == '/' && s[1] && s[1] != '/') bp = s + 1;
if (s > bp && s[0] == '/' && s[-1] != '/') ep = s;
++s;
}
if (!ep) ep = s;
*s = 0;
return bp;
}
Seems valid. Convoluted, but valid. Mishandles "\n".

<snip>

It is probably not intended to handle \n or non-printable characters,
but only sane file names. This assessment is based on some off-topic
knowledge.

The function could be simplified considerably by using the strrchr()
function.

<snip>

Which is indeed how the implementation I have works (not treating \n as
special). The implementation I have also does clever things to copy with
strange (but common) systems that do other annoying but off topic things.