strcat problem again

[Ian Stanley]

Hi,
Continuing my strcat segmentation fault posting-
I have a problem which occurs when appending two sting literals using
strcat.
I have tried to fix it by writing my own function that does the strcat
(mystract). Program below.
However this appears not to have fixed the problem and I don't know why it
shouldn't ?
Any further help as to what else I am doing wrong will be appreciated
regards
Ian.
The idea of the program is to:
#enter a integer to convert to words
#99 -- input
#ninety nine --output
Ps tried returning sprintf and using macros with no luck.

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
char* convertLessThanOneThousand(int number);
char* convert(int number);
char* mystrcat(char* dest, char* source);
static int num;
/*static char dest[1024];*/
static char *numNames[] = {
"",
" one",
" two",
" three",
" four",
" five",
" six",
" seven",
" eight",
" nine",
" ten",
" eleven",
" twelve",
" thirteen",
" fourteen",
" fifteen",
" sixteen",
" seventeen",
" eighteen",
" nineteen"
};
static char *tensNames[] = {
"",
" ten",
" twenty",
" thirty",
" forty",
" fifty",
" sixty",
" seventy",
" eighty",
" ninety"
};
char *majorNames[] = {
"",
" thousand",
" million",
" billion",
" trillion",
" quadrillion",
" quintillion"
};
int main(){
printf("enter a integer to convert to words ");
scanf("%d", &num);
printf("converted to words: %s\n", convert(num));
return 0;
}

char* convertLessThanOneThousand(int number) {
char* soFar;
/*char buffer[10000];
char* soFar;
soFar = buffer;*/
/* char result[256];*/
/*char* hundred = "hundred";*/
char hundred[] = "hundred";
if (number % 100 < 20){
soFar = numNames[number % 100];
number /= 100;
}
else {
soFar = numNames[number % 10];
number /= 10;

soFar = mystrcat(tensNames[number % 10], soFar);
number /= 10;
}
if (number == 0)
return soFar;
/*return numNames[number] + " hundred" + soFar;*/
/*sprintf(result, "%s%s%s", numNames[number], hundred, soFar);*/
return mystrcat(numNames[number], mystrcat(hundred, soFar));
}

char* convert(int number) {
char* zero = "zero";
if (number == 0) {
return zero; }
char* prefix = "";
char* soFar = "";
int place = 0;
do {
int n = number % 1000;
if (n != 0){
char* s = convertLessThanOneThousand(n);
/*soFar = s + majorNames[place] + soFar;*/
soFar = mystrcat(s, mystrcat(majorNames[place], soFar));
}
place++;
number /= 1000;
} while (number > 0);
/*return (prefix + soFar).trim();*/
return (mystrcat(prefix, soFar));
}

char* mystrcat(char* dest, char* source){
while(*dest){}
dest--;
while(*dest++ = *source++){}
return dest;
}

 

[Clemens Auer]

srtcat requires that the memory it should write to is already provided
... sounds like you're writing over your buffer and the system seg faults
...
i didn't read the full source .. so maybe i'm just writing nonsense..

Clemens

Hi,
Continuing my strcat segmentation fault posting-
I have a problem which occurs when appending two sting literals using
strcat.
I have tried to fix it by writing my own function that does the strcat
(mystract). Program below.
However this appears not to have fixed the problem and I don't know
why it shouldn't ?
Any further help as to what else I am doing wrong will be appreciated
regards
Ian.
The idea of the program is to:
#enter a integer to convert to words
#99 -- input
#ninety nine --output
Ps tried returning sprintf and using macros with no luck.

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
char* convertLessThanOneThousand(int number);
char* convert(int number);
char* mystrcat(char* dest, char* source);
static int num;
/*static char dest[1024];*/
static char *numNames[] = {
"",
" one",
" two",
" three",
" four",
" five",
" six",
" seven",
" eight",
" nine",
" ten",
" eleven",
" twelve",
" thirteen",
" fourteen",
" fifteen",
" sixteen",
" seventeen",
" eighteen",
" nineteen"
};
static char *tensNames[] = {
"",
" ten",
" twenty",
" thirty",
" forty",
" fifty",
" sixty",
" seventy",
" eighty",
" ninety"
};
char *majorNames[] = {
"",
" thousand",
" million",
" billion",
" trillion",
" quadrillion",
" quintillion"
};
int main(){
printf("enter a integer to convert to words ");
scanf("%d", &num);
printf("converted to words: %s\n", convert(num));
return 0;
}

char* convertLessThanOneThousand(int number) {
char* soFar;
/*char buffer[10000];
char* soFar;
soFar = buffer;*/
/* char result[256];*/
/*char* hundred = "hundred";*/
char hundred[] = "hundred";
if (number % 100 < 20){
soFar = numNames[number % 100];
number /= 100;
}
else {
soFar = numNames[number % 10];
number /= 10;

soFar = mystrcat(tensNames[number % 10], soFar);
number /= 10;
}
if (number == 0)
return soFar;
/*return numNames[number] + " hundred" + soFar;*/
/*sprintf(result, "%s%s%s", numNames[number], hundred, soFar);*/
return mystrcat(numNames[number], mystrcat(hundred, soFar));
}

char* convert(int number) {
char* zero = "zero";
if (number == 0) {
return zero; }
char* prefix = "";
char* soFar = "";
int place = 0;
do {
int n = number % 1000;
if (n != 0){
char* s = convertLessThanOneThousand(n);
/*soFar = s + majorNames[place] + soFar;*/
soFar = mystrcat(s, mystrcat(majorNames[place], soFar));
}
place++;
number /= 1000;
} while (number > 0);
/*return (prefix + soFar).trim();*/
return (mystrcat(prefix, soFar));
}

char* mystrcat(char* dest, char* source){
while(*dest){}
dest--;
while(*dest++ = *source++){}
return dest;
}

 

[Mark A. Odell]

Hi,
Continuing my strcat segmentation fault posting-
I have a problem which occurs when appending two sting literals using
strcat.
I have tried to fix it by writing my own function that does the strcat
(mystract). Program below.
However this appears not to have fixed the problem and I don't know why
it shouldn't ?

Is there *any* reason you can't fire up a debugger and single step through
this code? This is what I do when I simply cannot see my mistake.

 

[Chris Dollin]

Hi,
Continuing my strcat segmentation fault posting-
I have a problem which occurs when appending two sting literals using
strcat.

You can't concatenate string literals *at all*. To concatenate them,
you'd have to update the first one, but you're not permitted to update
a string literal. BOOM today.

Witness:

static char *numNames[] = {
"",
" one",
" two",
" three",
" four",
" five",
" six",
" seven",
" eight",
" nine",
" ten",
" eleven",
" twelve",
" thirteen",
" fourteen",
" fifteen",
" sixteen",
" seventeen",
" eighteen",
" nineteen"

return mystrcat(numNames[number], mystrcat(hundred, soFar));

char* mystrcat(char* dest, char* source){
while(*dest){}
dest--;
while(*dest++ = *source++){}
return dest;
}

If mystrcat worked at all [which it won't, since the ++ is missing in
the first while loop], you'd be updating one of the string literals
in numNames. No can do. You'll need to copy your literals into a
BIG ENOUGH buffer anyway.

Also witness:

char* convert(int number) {
char* zero = "zero";
if (number == 0) {
return zero; }
char* prefix = "";

You're either using C99 or C++, not the usual C, which doesn't allow
declarations following statements.

 

[Richard Bos]

However this appears not to have fixed the problem and I don't know why it
shouldn't ?

It doesn't fix the problem because, despite repeated hints and indeed
downright shoves and pushes in that direction, you seem incapable of
understanding that _one cannot write to a string literal_. Stop doing
that, and you've solved half of your problem.
Get your loops right and think about what you actually want to do, and
you just might solve the other half. But in any case, stop asking the
same question over and over again without really thinking about the
answers you get, because you won't solve anything _this_ way.

Richard

 

[Malcolm]

char* mystrcat(char* dest, char* source){
while(*dest){}
dest--;
while(*dest++ = *source++){}
return dest;
}

You missed a ++ on the first while loop.

In general, don't write this sort of C. You are using C idioms that are
meaningless to anyone not fluent in the language. Even for those who know C
well, this sort of cryptic code is hard to read and dry run.

How about

char *mystrcat(char *dest, char *source)
{
while(*dest)
dest++;

do
{
*dest = *source;
dest++;
} while(*source++);

return dest;
}

 

[Nick Austin]

Hi,
Continuing my strcat segmentation fault posting-
I have a problem which occurs when appending two sting literals using
strcat.
[snip]

char* mystrcat(char* dest, char* source){
while(*dest){}
dest--;
while(*dest++ = *source++){}
return dest;
}

This is UB. You cannot write to a string literal. You have
to allocate a new buffer for the newly created string thus:

char* mystrcat( char* dest, char* source )
{
char *newstring;

newstring = malloc( strlen( dest ) + strlen( source ) + 1 );

if ( newstring )
{
strcpy( newstring, dest );
strcat( newstring, source );
}
return newstring;
}

Nick.

 

[Paul Hsieh]

Is there *any* reason you can't fire up a debugger and single step through
this code? This is what I do when I simply cannot see my mistake.

The debugger tells them that the error is on (*dst++ = *src++) ... dst
is as expected as is src. How does the debugger help in this case?

 

[Mark A. Odell]

(e-mail address removed) (Paul Hsieh) wrote in

The debugger tells them that the error is on (*dst++ = *src++) ... dst
is as expected as is src. How does the debugger help in this case?

How? You're a kidding I assume. If not, let's investigate this. Either
writing to dst is at fault or reading from src is. A quick read of src
with the debugger shows that not to be a problem. Then we look at what dst
points to. Hmm. Funny, it seems to be in the same area as the code (on
this implementation) maybe I am not able to write to that memory region.
Okay, dst points to my list of string literals. I wonder if you can't
write to string literals. Let me make a tiny program with a string literal
and a normal array and try writing to each memory region. I'll use my
debugger again to see which write barfs. Wow, the write to the string
literal. Okay so I can't write to string literals. Now I will change my
program such that I don't do this anymore.

 

[Nick Austin]

The debugger tells them that the error is on (*dst++ = *src++) ... dst
is as expected as is src. How does the debugger help in this case?

That statement performs several operations. So you replace that line
with separate statements for reading, writing and the increments:

{
char tmp;

tmp = *src;
*dst = tmp;
dst++;
src++;
}

Recompile and run again with the debugger. This tells you that
the problem is in the line *dst = tmp so you can conclude that
you are not allowed to write to that area of memory.

Nick.

 

[Mark McIntyre]

That statement performs several operations. So you replace that line
with separate statements for reading, writing and the increments:

It truly astonishes me that they don't seem to teach this stuff in
schools any more. Are people and their teachers so reliant on their
debugger to be all-seeing that they've lost the most elementary basic
skills of debugging?

 

[Randy Howard]

It truly astonishes me that they don't seem to teach this stuff in
schools any more. Are people and their teachers so reliant on their
debugger to be all-seeing that they've lost the most elementary basic
skills of debugging?

You don't even need to know how to use a debugger if you can get
free answers to homework problems on Usenet. No wonder young
candidates are so easy to integrate into a team and start
making an impact quickly. *cough*

 

[Karl Heinz Buchegger]

It truly astonishes me that they don't seem to teach this stuff in
schools any more. Are people and their teachers so reliant on their
debugger to be all-seeing that they've lost the most elementary basic
skills of debugging?

The problem is that in these days most teachers haven't written anything more
then simple toy programs. Developing debug skills is no longer essential since
changing a toy program at random until it works is easy and fast today.

 

[Martijn Lievaart]

Hi,
Continuing my strcat segmentation fault posting-
I have a problem which occurs when appending two sting literals using
strcat.
I have tried to fix it by writing my own function that does the strcat
(mystract). Program below.
However this appears not to have fixed the problem and I don't know why it
shouldn't ?
Any further help as to what else I am doing wrong will be appreciated
regards
Ian.

You are still changing string literals. Your problem is still exactly the
same as in your first posting. You need to step back, reread all answers
to the posts and start to really understand what the problem is.

Your problem is twofold:

- You have some assumptions about how C-strings work. These assumptions
are incorrect.
- Based on your assumptions your program has a certain stucture. But as
the assumptions are incorrect, your structure is incorrect. The tinkering
you are doing now will /never/ produce a working program.

The problem why your program segfaults is this.

char *p="some string";

"some string" has actually type const char* that is converted to a char*.
Normally this conversion is not allowed, but in this context the standards
make an exception as this idiom is very often used. But that does not make
it correct. In fact, every program that has a line like the above has a
bug. Your program has multiple such lines, as you'll see shortly.

You are /not/ allowed to modify what p points to. It is constant memory.
Even if it seems to work sometimes. You are CERTAINLY not allowed to
strcat to this memory. This is what you have been doing from the first
version you posted to the last. Exactly the same.

Try this:
const char *p1="some string";
char *p=p1;

It does not compile. You cannot convert a const char* to a char*, it would
loose an important property, the const. This is correct, and the previous
example is a hole in the type system that has produced many, many
segfaults.

This also holds for
char *a[] = {"1", "2"};

This should be
const char *a[] = {"1", "2"};

Th first compiles, but is a dissaster waiting to happen, as you found out.
It is the same as the 'char *p="some string"' I talked about, only now
disguised as an array.

So what about your problem? Well let's start with taking that bug out.
Then the compiler will start giving you errors about conversions from
const char* to char*. This will not solve your problem as in "now my
program runs", it solves your problem as in "now my problem is caught by
the compiler and I can start solving it".

static char *numNames[] = {

static const char *numNames[] = {

static char *tensNames[] = {

static const char *tensNames[] = {

char *majorNames[] = {

const char *majorNames[] = {

Make these changes and see what happens. Your compiler should pinpoint the
problem exactly now.

Next, how to solve it.

What you try to do is some thing like:

int main(){
printf("enter a integer to convert to words ");
scanf("%d", &num);
printf("converted to words: %s\n", convert(num));
return 0;
}

string convertLessThanOneThousand(int number) {
....
}

string convert(int number) {
....
string s = convertLessThanOneThousand(n);
....
}

C does NOT work this way. You can do this, but only in a roundabout way.
As long as you try to do this, without understanding why it doesn't work
in C, your program will /never/ work.

General strategies you should follow:

1) use a caller supplied buffer.

void f(char *buf)
{
strcpy(buf, "hello world");
}

int main()
{
char buf[1024];
f(buf);
/* buf is now filled, use buf */
puts(buf);
}

2) Use dynamic allocation

char *f()
{
char *p=malloc(sizeof("hello world"));
strcpy(p, "hello world");
return p;
}

int main()
{
char *p = f();
/* we now 'own' the memory and must release it later */
puts(p);
free(p);
}

3) return const char *, however, you cannot modify this. Use this together
with strategy 1 and 2 to solve your problem.

static const char *numNames[] = {
" zero",
" one",
" two",
" three",
" four",
" five",
" six",
" seven",
" eight",
" nine",
" ten"
};

const char * to_name(int num)
{
if (num<0 || num >19) {
/* start bitching and exit program */
}
return numNames[num];
}

Do NOT do:

1) modify constant memory:

char *p = "hello";

strcat(p, ", world");

This is what you still are doing in your program, even if it is hidden
under some layers. You are trying things, but you will never get a working
program if you do not step back and understand that this is what you are
doing.

Instead, use:

const char *p="hello";

or

char p[SOMEVALUE]="hello";

In the second case, you can modify the contents of p. But only up
to SOMEVALUE bytes. And once changed, stays changed, so you probably do
not want this in your program.

Stop thinking about char* as a string type, and start thinking about a
char* as pointer to some memory. That blob of memory has a size, and a
lifetime. Once you understand this, and only if you understand this, will
you be able to solve your problem.

You probably need to get a good book on C. You also probably need to step
back and start with something simpler than this.

HTH,
M4

 

[Mark McIntyre]

You don't even need to know how to use a debugger if you can get
free answers to homework problems on Usenet.

TANSTAAFL. They pay later, when I employ them... bwahahaha.

I did like your sig tho. Those poor SCO people...

 

[Paul Hsieh]

How? You're a kidding I assume. If not, let's investigate this. Either
writing to dst is at fault or reading from src is.

Yes, but the debugger doesn't really tell them that. It just says
there is a write fault on this line. A typical programmer is going to
assume that this means that dst is pointing into la-la land. They
examine the dst pointer in the debugger and don't see anything wrong.

[...] A quick read of src
with the debugger shows that not to be a problem. Then we look at what dst
points to. Hmm. Funny, it seems to be in the same area as the code (on
this implementation) maybe I am not able to write to that memory region.

Actually, in many implementations the pointer appear in data space.
Under both MSVC++ and WATCOM C/C++ on Windows the pointers will appear
in the _INIT segment. It faults under MSVC and just blasts away under
WATCOM C/C++. Examining the pointer, the programmer all of a sudden
needs to know what the hell the _INIT segment is to understand what is
going on.

Okay, dst points to my list of string literals. I wonder if you can't
write to string literals.

Does the debugger tell you this, or is this a leap of logic? (Go back
to my original question.)

 

[Paul Hsieh]

The debugger tells them that the error is on (*dst++ = *src++) ... dst
is as expected as is src. How does the debugger help in this case?

[...] This tells you that
the problem is in the line *dst = tmp so you can conclude that
you are not allowed to write to that area of memory.

Well, you conclude that if it ever occurrs to you that portions of
data in a program you wrote/compiled are not writable. Not surprising
if you are aware of such things a priori. Not going to happen if you
are a junior programmer who has not found any source of any
programming material written anywhere that explains the C language
that explains things like this. Notice that the original poster *DID*
rewrite his code just for debugging purposes and still felt the need
to post here.

 

[Dave Thompson]

You are still changing string literals. Your problem is still exactly the
same as in your first posting. You need to step back, reread all answers
to the posts and start to really understand what the problem is.
Right.

The problem why your program segfaults is this.

char *p="some string";

"some string" has actually type const char* that is converted to a char*.
Normally this conversion is not allowed, but in this context the standards
make an exception as this idiom is very often used. <snip>

Not quite. In standard C, a string literal has type (unqualified)
char [], and (like any array) decays, to char *; for compatibility
with/transition from pre-standard C it is _not_ const although it _is_
illegal to write to it. In C++, it has type const char [], but there
is as you say a special (and deprecated) conversion to char *.
(And a wide string literal similarly with wchar_t.)

You are /not/ allowed to modify what p points to. It is constant memory.
Even if it seems to work sometimes. You are CERTAINLY not allowed to
strcat to this memory. This is what you have been doing from the first
version you posted to the last. Exactly the same.

Right. In both languages, a string literal produces an array just big
enough to contain its value. It is illegal in both languages to store
(or access at all) past the end of an array, as strcat() tries to do
if the second argument is other than an empty string.

Next, how to solve it.

1) use a caller supplied buffer.

void f(char *buf)
{
strcpy(buf, "hello world");
}

int main()
{
char buf[1024];
f(buf);
/* buf is now filled, use buf */
puts(buf);
}

Yes, but unless you know that the buffer/string size limit is the same
for all possible callers and won't change, for example is constrained
by an external device, often better to pass the size explicitly and
use it to check for and prevent overflow.

2) Use dynamic allocation

char *f()
{
char *p=malloc(sizeof("hello world"));
strcpy(p, "hello world");
return p;
}

Yes. In C, slightly better to use (void) to make it a prototype, with
checking of calls; in C++ () and (void) are the same. And should
check malloc's return value for non-null before using it.

int main()
{
char *p = f();
/* we now 'own' the memory and must release it later */
puts(p);
free(p);
}

3) return const char *, however, you cannot modify this. Use this together
with strategy 1 and 2 to solve your problem.

<snip>

The usual 3 is to use space (an array) declared static in the callee,
or otherwise static (file-scope internal, or "global" external), and
return a pointer to that. Doesn't need to be freed, but if/as long as
you keep pointer copies the value they point to may/will change if the
routine (or perhaps another routine sharing the variable) is called by
any other code, including other code you call (like a library), or by
another thread if you go outside standard C *or* C++ to add threads.

And in C++ only, of course, 0 should be to use std::string.

Stop thinking about char* as a string type, and start thinking about a
char* as pointer to some memory. That blob of memory has a size, and a
lifetime. Once you understand this, and only if you understand this, will
you be able to solve your problem.

Exactly.

- David.Thompson1 at worldnet.att.net

 

[Martijn Lievaart]

On Fri, 19 Sep 2003 13:53:32 +0200, Martioften used.

Not quite. In standard C, a string literal has type (unqualified)
char [], and (like any array) decays, to char *; for compatibility
with/transition from pre-standard C it is _not_ const although it _is_
illegal to write to it. In C++, it has type const char [], but there
is as you say a special (and deprecated) conversion to char *.
(And a wide string literal similarly with wchar_t.)

I just learned this myself. Thought C was the same as C++ here. Doesn't
matter though, the net effect is the same in the end. You should not write
to string literals, even if the compiler does not complain.

int main()
{
char buf[1024];
f(buf);
/* buf is now filled, use buf */
puts(buf);
}

Yes, but unless you know that the buffer/string size limit is the same
for all possible callers and won't change, for example is constrained
by an external device, often better to pass the size explicitly and
use it to check for and prevent overflow.

Yes. I deliberately did not mention this as the focus was on who supplies
the memory. In retrospect, I should have passed the size.

Yes. In C, slightly better to use (void) to make it a prototype, with
checking of calls; in C++ () and (void) are the same. And should
check malloc's return value for non-null before using it.

Yuck! I'm to use to C++ to answer C questions. Yes, there should be a
'void' there.

I deliberately did not check the return value of malloc, the OP seems to
have some difficulty in absorbing simple concepts so I wanted not to make
it any more complecated than nessecery. I was ready to add this check once
he tried to use malloc though.

M4