NULL character assignment not working in C program

[Curtis Deese]

We are using the following version of cc on HP-UX 11.31 Itanium: cc: HP C/aC++ B3910B A.06.25 [Nov 30 2009]

I have the following function:

/*----------trim (char) c from right-side of string *p------------------*/
char *strtrim_right( char *p)
{
char *end;
int len;

len = strlen( p);
while ( *p && len)
{
WriteDebug("In strtrim_right 2 p='%s'", p);

end = p + len-1;
WriteDebug("In strtrim_right 2 end='%s'", end);

if(' ' == *end)
{
WriteDebug("In strtrim_right 3");
p[len-1] = '\0';
WriteDebug("In strtrim_right 4");
}
else
break;

len = strlen( p);
}
WriteDebug("In strtrim_right 5");

return( p);
}

Here is the output at the end of the log file:

25481-12:58:41.489-dbg In strtrim_right 2 p='Server Assigned. '
25481-12:58:41.489-dbg In strtrim_right 2 end=' '
25481-12:58:41.489-dbg In strtrim_right 3

Notice that it never gets to 'In strtrim_right 4'.

Why is my program terminating at the line p[len-1] = '\0'; ?

It also terminates the same way if I use p[len-1] = NULL;

How do I resolve the problem?

[James R. Ferguson]

Hi:

When you inject a null byte into your string you are effectively terminating the string. Strings are arrays of characters that are terminated by a NULL byte.

The 'strlen()' function returns the number of characters in the string argument, not including the terminating null byte.

Regards!

...JRF...

[Curtis Deese]

I know that a NULL character terminates a string and that a string is an array of characters. I am trying to terminate a string with a NULL character. Why does my program crash when I try to do that?

[TwoProc]

I always terminated my strings with "(char)0".

Main problem, you're not decrementing "len" inside of each loop, so you never rollback in the string enough to find the space character.

You're not decrementing len over each loop, which I'm a bit surprised at, since it's the driver for the while loop. Also, why fool with *end and with trying to move len around (and then somehow not doing it)?

Anyways, just for giggles, I wrote one, but haven't tested it - feel free to ignore.

char *strtrim_right(char *p)
{
int len = strlen(p);

while (len)
{
!(*(p+len)-(char)32)?
break:
len--;
}
len>0 ? *(p+len)=(char)0 : 0;
return (p);
}

[Steven Schweda]

> I know that a NULL character [...]

We normally refer to a NUL character and a
NULL pointer. But I digress.

> [...] Why does my program crash when I try
> to do that?

We non-psychics can't know why your program
crashes, because WE CAN'T SEE YOUR PROGRAM.
All we can see is an unbuildable fragment of
a program. If you want good free advice,
then you should make it easier for your
advisers to figure out what you're doing.

If I try to make what seems like a plausible
program out of your program fragment, then I
can _guess_ what your program may be doing.
In _my_ program, your function works once,
and fails once. Here's what my program does:

alp $ cc wd /list /show = (all, nomess)
alp $ link wd
alp $ run wd
Before (1): >Server Assigned. <
In strtrim_right 2 p='Server Assigned. '
In strtrim_right 2 end=' '
In strtrim_right 3
In strtrim_right 4
In strtrim_right 2 p='Server Assigned.'
In strtrim_right 2 end='.'
In strtrim_right 5
After (1): >Server Assigned.<
Before (2): >Server Assigned.<
In strtrim_right 2 p='Server Assigned. '
In strtrim_right 2 end=' '
In strtrim_right 3
%SYSTEM-F-ACCVIO, access violation, reason mask=04, virtual address=000000000001
01C8, PC=0000000000020158, PS=0000001B
%TRACE-F-TRACEBACK, symbolic stack dump follows
image module routine line rel PC abs PC
wd WD strtrim_right 2256 0000000000000158 0000000000020158
wd WD main 2276 00000000000002FC 00000000000202FC
wd WD __main 2270 0000000000000234 0000000000020234
0 FFFFFFFF8037FCE4 FFFFFFFF8037FCE4
%TRACE-I-END, end of TRACE stack dump

Line 2256 in the listing is the obvious one:

3 2256 p[len-1] = '\0';

alp $ help /mess /faci = system ACCVIO
[...]
The reason mask is
a longword whose lowest 5 bits, if set, indicate that the
instruction caused a length violation (bit 0), referenced the
process page table (bit 1), attempted a read/modify operation
(bit 2), was a vector operation on an improperly aligned
vector element (bit 3), or was a vector instruction reference
to an I/O space address (bit 4).
[...]

"reason mask=04" has bit 2 set, which means,
"attempted a read/modify operation".

My (actual, whole, real) program looks like
this:

alp $ type wd.c
#include
#include
#include

int WriteDebug( char *fmt, ...)
{
va_list ap;
int n;

va_start( ap, fmt);
n = vprintf( fmt, ap);
n += printf( "\n");
return n;
}


/*----------trim (char) c from right-side of string *p------------------*/
char *strtrim_right( char *p)
{
char *end;
int len;

len = strlen( p);
while ( *p && len)
{
WriteDebug("In strtrim_right 2 p='%s'", p);

end = p + len-1;
WriteDebug("In strtrim_right 2 end='%s'", end);

if(' ' == *end)
{
WriteDebug("In strtrim_right 3");
p[len-1] = '\0';
WriteDebug("In strtrim_right 4");
}
else
break;

len = strlen( p);
}
WriteDebug("In strtrim_right 5");

return( p);
}


int main ( void)
{

}
char str[] = "Server Assigned. ";

printf( "Before (1): >%s<\n", str);
strtrim_right( str);
printf( "After (1): >%s<\n", str);

printf( "Before (2): >%s<\n", str);
strtrim_right( "Server Assigned. ");
printf( "After (2): >%s<\n", str);
}

In my environment ...
alp $ cc /version
HP C V7.3-009 on OpenVMS Alpha V8.3
... I get a friendly error message with a
traceback. Yours is less convenient, but I
suspect that the problem there is the same as
the problem here. Did you get a "core" file?
Have you learned how to use a debugger with a
core file? It's a good skill to have on a
UNIX(-like) system.

[Steven Schweda]

> Main problem, [...]

Not really. (On many levels.)

> You're not decrementing len [...]

len = strlen( p);

(The second one.)

As usual, it's easier to explore an actual
program than it is a listing of a program
fragment. In fact, while probably not
ideally efficient, this thing does handle
multiple trailing spaces. For example:

alp $ run wd2
Before (1): >Server Assigned. <
In strtrim_right 2 p='Server Assigned. '
In strtrim_right 2 end=' '
In strtrim_right 3
In strtrim_right 4
In strtrim_right 2 p='Server Assigned. '
In strtrim_right 2 end=' '
In strtrim_right 3
In strtrim_right 4
In strtrim_right 2 p='Server Assigned. '
In strtrim_right 2 end=' '
In strtrim_right 3
In strtrim_right 4
In strtrim_right 2 p='Server Assigned.'
In strtrim_right 2 end='.'
In strtrim_right 5
After (1): >Server Assigned.<

What it can _not_ handle is a read-only
string argument.


> [...] (char)32

Not much on portable code are we? ASCII is
not the whole world. "' '" was better.

http://en.wikipedia.org/wiki/Extended_Binary_Coded_Decimal_Interchange_Code

[Steven Schweda · Accepted solution]

Less stupid main program:

int main( void)
{
char str1[] = "Server Assigned. ";
char *str2 = "Server Assigned. ";

printf( "Before (1): >%s<\n", str1);
strtrim_right( str1);
printf( "After (1): >%s<\n", str1);

printf( "Before (2): >%s<\n", str2);
strtrim_right( str2);
printf( "After (2): >%s<\n", str2);
}


(Essentially the same results, just better
clarity and diagnostic messages.)


> [...] I wrote one, [...]

I don't really like code this compact --
too hard to add in those printf's -- but it's
an exercise.

char *strtrim_right( char *p)
{
int len;

for (len = strlen( p);
(len > 0) && (*(p+ (--len)) == ' ');
*(p+ len) = '\0');

return p;
}

Or, if you prefer subscripts to pointer
dereferencing:

for (len = strlen( p);
(len > 0) && (p[ --len] == ' ');
p[ len] = '\0');

As usual, many things are possible.


> [...] haven't tested it [...]

Testing, of course, is easier with a whole,
working program.

[Dennis Handly]

>Steven: What it can _not_ handle is a read-only string argument.
char str1[] = "Server Assigned. ";
char *str2 = "Server Assigned. ";

Right. The first will work and the second abort (using the default options for +Olit=).

[TwoProc]

IMHO best one is Steven's below as the cleanest:

for (len = strlen( p);
(len > 0) && (p[ --len] == ' ');
p[ len] = '\0');

[Steven Schweda]

> IMHO best one is [...]

"best" is not well-defined here. Note that
that "for" loop stores a NUL over every
trailing space character, not only the
left-most one, so, while the code may be
compact, it may run more slowly than smarter
code which stores only the one required NUL
(in the right place). And, the speed of any
such code may depend on the input data --
more or fewer trailing spaces. Everything's
complicated.

[TwoProc]

Right, so I commented on which code frag I liked, and that one was it.

Reason: It was very small, very tight, yet easy to read. I'd gladly take the extra stores to memory for all of the stack loads and pops for all the subbys in the other extremum of code demonstrated. Also with your code, I'd bet that the optimizer would stick and run those values out of a register for that code segment regardless, so I don't think it would hurt at all.

I liked the code I posted myself for being quick, small and tight, but I didn't like it all for readability.

Your code was a much nicer mix of both (tight yet readable).

You **could** have left off the "Less stupid main program" comment at the beginning, just in case you wanted to let folks mistakenly think you're a nice guy for a minute or two, but your code response was very good nonetheless. :-)

[Steven Schweda]

> You **could** have left off the "Less
> stupid main program" comment [...]

I could have, but it seemed to me to be
appropriate. It referred to my own original
main program. If anyone else supplied one,
then I missed it. The copy+paste/typo
problem in my original main program (spurious
"}") was not its only defect. I'm sure that
if the ITRC Thought Police determine that I'm
guilty of inflicting personal abuse on
myself, then they'll take appropriate action.

[Dennis Handly]

> TwoProc: I'd gladly take the extra stores to memory for all of the stack loads and pops

There is no stack machine here, RISC and EPIC are register machines.

>I'd bet that the optimizer would stick and run those values out of a register for that code segment regardless, so I don't think it would hurt at all.

You have too much faith in the optimizer. ;-) You told it to do a store to that pointer, it doesn't know strings are special or that it even is a string.

[TwoProc]

Dennis - Are you sure?

I know RISC design has lots o' registers so that the compiler optimizer can stick as much stuff as possible in registers. But, no stack?

And I know that the "no stack" theory has been part of RISC design for a long time, but in truth, there certainly are stacks, and the little tight code that Steven submitted almost absolutely avoids the necessity of using them.

I don't believe that there's a decent way to put together a C compiler that doesn't use a stack, unless we just rule out regression as method of programming, or we stop programs when the registers fill up.

$> kctune | grep maxssiz

for a good look.

[Dennis Handly]

>Are you sure?
>no stack?

Yes. I said "stack machine", not no stack. The stack is simulated on register machines.

>there certainly are stacks, and the little tight code that Steven submitted almost absolutely avoids the necessity of using them.

The compiler cares about sequences of frames, not really stacks. Steven's code only stores through that pointer p in strtrim_right. There is no local frame, only a RSE frame on Integrity.

>unless we just rule out regression as method of programming

You mean recursion.

$> kctune maxssiz # for a good look.

This is that simulated stack.

[TwoProc]

>Are you sure?
>no stack?

Yes. I said "stack machine", not no stack. The stack is simulated on register machines.

Yep, I just read up on this. Withdrawn. It's definitely there as you say.

>there certainly are stacks, and the little tight code that Steven submitted almost absolutely avoids the necessity of using them.

The compiler cares about sequences of frames, not really stacks. Steven's code only stores through that pointer p in strtrim_right. There is no local frame, only a RSE frame on Integrity.

> yep, we've agreed on the fact that Steven's code is not gonna use the stack three or four time snow.

>unless we just rule out regression as method of programming

You mean recursion.

Yep, goofball moment - recursion.

$> kctune maxssiz # for a good look.

This is that simulated stack.

Ok, this point, I need to read up on more. But I think I see what you're saying in that whole frames (holding a stack) get moved in and out and then the whole thing is processed locally in the stack register machine segment of the cpu. And then possibly parked back later. Thanks for the correction.

[Dennis Handly]

>But I think I see what you're saying in that whole frames (holding a stack) get moved in and out and then the whole thing is processed locally in the stack register machine segment of the CPU. And then possibly parked back later.

Possibly you are reading more into the Register Save Engine?

Basically there are two "stacks". One is for the user data and one is for the register frames. The RSE automatically spills and fills based on the size of the register frame, on call entry and exits.

[Laurent Menase]

Just for fun and addition misers
char *strtrim_right( char *p)
{
register char *q;

for (q = p+ strlen( p);
(q!=p) && *(--q) == ' ');
*(q) = '\0');

return p;
}

or to avoid the *(q)=0 on every steps
and have a move reg to reg in place of a store one char.
char *strtrim_right( char *p)
{
register char *q,*z;

for (z = q = p+ strlen( p);
(q!=p) && *(--q) == ' ');
z=q);
*(z) = '\0');

return p;
}

[Dennis Handly]

>Laurent: Just for fun and addition misers

You violated the law of conservation of parenthesis and anti-parenthesis. :-)

[Laurent Menase]

Yes Denis, you are right, being miser I wasted parenthesis:
it is
char *strtrim_right( char *p)
{
register char *q;

for (q = p+ strlen( p);
(q!=p) && (*--q == ' ');
*q = '\0');

return p;
}
or
char *strtrim_right( char *p)
{
register char *q,*z;

for (z = q = p+ strlen( p);
(q!=p) && ( *--q == ' ');
z=q);
*z = '\0';

return p;
}

or now with only one walk from left to right:
char *strtrim_right( char *p)
{
register char *q,*z;
register char c;
for (z=0,q=p ;(c=*q);q++)
{
for(;(c=*q)&&(c!=' ');q++);
for(z=q;(c=*q)&&(c==' ');q++);
}
*z = '\0';
return p;
}