Struggling with libraries

[Tony Burrows]

I'm just learning C as another language, and I'm trying to build some
utilities into a library. I have this (crude I know) function:

void insert(char* insrt, char* source, int place){
char temp[strlen(insrt)+strlen(source)+1]; strncpy(temp, source, place);
temp[place]='\0';
strcat(temp, insrt);
strcat(temp, source+place);
strcpy(source, temp);
}

In a simple file, it all works, but in a static library, when I call it I
get a segnmentation fault. The villain is the final strcpy.

I tried returning temp instead and that doesn't work either, I don't get
quite the string that exists in the function (and the compiler warns about
returning a local variable).

I know there's a solution, I just have no idea what it is. Any help would
be great.

Tony

 

[=?ISO-8859-1?Q?=22Nils_O=2E_Sel=E5sdal=22?=]

I'm just learning C as another language, and I'm trying to build some
utilities into a library. I have this (crude I know) function:

void insert(char* insrt, char* source, int place){
char temp[strlen(insrt)+strlen(source)+1]; strncpy(temp, source, place);
temp[place]='\0';
strcat(temp, insrt);
strcat(temp, source+place);
strcpy(source, temp);
}

In a simple file, it all works, but in a static library, when I call it I
get a segnmentation fault. The villain is the final strcpy.

I tried returning temp instead and that doesn't work either, I don't get

quite the string that exists in the function (and the compiler warns about
returning a local variable).

It's illegal to return pointers to auto objects.

I know there's a solution, I just have no idea what it is. Any help would
be great.

WIthout seeing how it is beeing called, hard to tell.

Be very very sure your 'source' has enough room for
'insrt'.

 

[Default User]

I'm just learning C as another language, and I'm trying to build some
utilities into a library. I have this (crude I know) function:

void insert(char* insrt, char* source, int place){
char temp[strlen(insrt)+strlen(source)+1]; strncpy(temp, source,
place); temp[place]='\0';
strcat(temp, insrt);
strcat(temp, source+place);
strcpy(source, temp);
}

In a simple file, it all works, but in a static library, when I call
it I get a segnmentation fault. The villain is the final strcpy.

I tried returning temp instead and that doesn't work either, I don't
get quite the string that exists in the function (and the compiler
warns about returning a local variable).

I know there's a solution, I just have no idea what it is. Any help
would be great.

You need to help us. Post a COMPLETE minimal program that demonstrates
the problem. I all likelihood, there is not sufficient room in source
to contain the expanded string, but how can we tell?




Brian

 

[Fred Kleinschmidt]

I'm just learning C as another language, and I'm trying to build some
utilities into a library. I have this (crude I know) function:

void insert(char* insrt, char* source, int place){
char temp[strlen(insrt)+strlen(source)+1]; strncpy(temp, source, place);
temp[place]='\0';
strcat(temp, insrt);
strcat(temp, source+place);
strcpy(source, temp);

Crash! source is not large enough to hold the new characters.
You need to allocate or reallocate space. What you need is:

char * insert(char* insrt, char* source, int place){
char *temp=NULL;
size_t nch = strlen(insrt)+strlen(source)+1;
temp = malloc(nch);
if ( temp ) {
strncpy(temp, source, place);
temp[place]='\0';
strcat(temp, insrt);
strcat(temp, source+place);
}
return temp;
}

Or you could reallocate source

void insert(char* insrt, char** source, int place){
char *temp=NULL;
size_t nch = strlen(insrt)+strlen(source)+1;
temp = realloc(*source, nch);
if ( temp ) {
strncpy(temp, source, place);
temp[place]='\0';
strcat(temp, insrt);
strcat(temp, source+place);
}
else {
/* Decide what to do if the realloc fails */
...
}
}

 

[Tony Burrows]

I'm just learning C as another language, and I'm trying to build some
utilities into a library. I have this (crude I know) function:

void insert(char* insrt, char* source, int place){
char temp[strlen(insrt)+strlen(source)+1]; strncpy(temp, source, place);
temp[place]='\0';
strcat(temp, insrt);
strcat(temp, source+place);
strcpy(source, temp);
}

In a simple file, it all works, but in a static library, when I call it I
get a segnmentation fault. The villain is the final strcpy.

I tried returning temp instead and that doesn't work either, I don't get

quite the string that exists in the function (and the compiler warns about
returning a local variable).

It's illegal to return pointers to auto objects.

I know there's a solution, I just have no idea what it is. Any help would
be great.

WIthout seeing how it is beeing called, hard to tell.

Be very very sure your 'source' has enough room for
'insrt'.

I've called it from a simple little program:

#include <stdio.h>
#include "insert.h"

int main(void){
char *str1 = "You are a nutcase!";
char *str2 = "big ";
puts(str1);
puts(str2);
insert(str2, str1, 10);
return 0;
}

insert.h is
#include <string.h>
void insert(char* insrt, char* source, int place);
void insertn(char* insrt, char* source, int place, int size);

Compiled with gcc -o testInsert testInsert.c insert.c

If I put print statements in, then the segmentation fault happens in the
insert function when strcpy(source, temp) is executed.

If I created a string on the heap with (I think) malloc and returned a
pointer to that, would that deal with the problem? If so, is it the right
way? The C programming books don't seem to touch on this.

Tony

 

[Default User]

I've called it from a simple little program:

#include <stdio.h>
#include "insert.h"

int main(void){
char *str1 = "You are a nutcase!";
char *str2 = "big ";
puts(str1);
puts(str2);
insert(str2, str1, 10);
return 0;
}

insert.h is
#include <string.h>
void insert(char* insrt, char* source, int place);
void insertn(char* insrt, char* source, int place, int size);

Compiled with gcc -o testInsert testInsert.c insert.c

If I put print statements in, then the segmentation fault happens in
the insert function when strcpy(source, temp) is executed.

You have two problems. One is that string literals are not modifiable.
Passing str1 to a function that modifies it is undefined behavior. For
you, luckily, that UB was a crash and alerted you to the problem.

The second problem is that even if it was modifiable, str1 is an array
of 18 characters, with no place to put in any extras.

If I created a string on the heap with (I think) malloc and returned a
pointer to that, would that deal with the problem? If so, is it the
right way? The C programming books don't seem to touch on this.

Inside the insert() function? That's one way to do it. Some people
don't like to have functions that allocate memory and leave the
responsibility to deallocate to the caller. The other way is to pass in
a buffer large enough to hold the resulting string.

void insert(const char* insrt, const char* source, char *dest, int
place);

Allocate dest in the calling function, with its size equal to the
string length of insrt + source + 1.



Brian

 

[Keith Thompson]

I'm just learning C as another language, and I'm trying to build some
utilities into a library. I have this (crude I know) function:

void insert(char* insrt, char* source, int place){
char temp[strlen(insrt)+strlen(source)+1]; strncpy(temp, source, place);
temp[place]='\0';
strcat(temp, insrt);
strcat(temp, source+place);
strcpy(source, temp);
}

[snip]
I've called it from a simple little program:

#include <stdio.h>
#include "insert.h"

int main(void){
char *str1 = "You are a nutcase!";
char *str2 = "big ";
puts(str1);
puts(str2);
insert(str2, str1, 10);
return 0;
}

insert.h is
#include <string.h>
void insert(char* insrt, char* source, int place);
void insertn(char* insrt, char* source, int place, int size);

Two problems. First, you're attempting to modify a string literal.
Second, even if the string literal were modifiable (as it might be on
some implementations), you haven't allocated enough space to hold the
result.

 

[Flash Gordon]

I'm just learning C as another language, and I'm trying to build some
utilities into a library. I have this (crude I know) function:

void insert(char* insrt, char* source, int place){
char temp[strlen(insrt)+strlen(source)+1]; strncpy(temp, source, place);
temp[place]='\0';
strcat(temp, insrt);
strcat(temp, source+place);
strcpy(source, temp);
}

In a simple file, it all works, but in a static library, when I call it I
get a segnmentation fault. The villain is the final strcpy.

I tried returning temp instead and that doesn't work either, I don't get
quite the string that exists in the function (and the compiler warns about
returning a local variable).

It's illegal to return pointers to auto objects.

I know there's a solution, I just have no idea what it is. Any help would
be great.

WIthout seeing how it is beeing called, hard to tell.

Be very very sure your 'source' has enough room for
'insrt'.

I've called it from a simple little program:

#include <stdio.h>
#include "insert.h"

int main(void){
char *str1 = "You are a nutcase!";
char *str2 = "big ";
puts(str1);
puts(str2);
insert(str2, str1, 10);
return 0;
}

insert.h is
#include <string.h>
void insert(char* insrt, char* source, int place);
void insertn(char* insrt, char* source, int place, int size);

Compiled with gcc -o testInsert testInsert.c insert.c

If I put print statements in, then the segmentation fault happens in the
insert function when strcpy(source, temp) is executed.

If I created a string on the heap with (I think) malloc and returned a
pointer to that, would that deal with the problem? If so, is it the right
way? The C programming books don't seem to touch on this.

I'm sure they do. The comp.lang.c FAQ certainly does with a much simpler
example here http://c-faq.com/strangeprob/strlitnomod.html which tells
you that string literals are not modifiable.

In addition, even if string literals were modifiable, you are trying to
make the string longer, so this would not work either:
#include <stdio.h>
#include "insert.h"

int main(void){
char str1[] = "You are a nutcase!";
char *str2 = "big ";
puts(str1);
puts(str2);
insert(str2, str1, 10);
return 0;
}

Since it would go off the end of str1.

If I was writing a function like your insert I would probably make is
malloc enough space for a new string and return the resultant string in
that rather than modifying one of the input strings.

 

[Flash Gordon]

I'm just learning C as another language, and I'm trying to build some
utilities into a library. I have this (crude I know) function:

void insert(char* insrt, char* source, int place){
char temp[strlen(insrt)+strlen(source)+1]; strncpy(temp, source, place);
temp[place]='\0';
strcat(temp, insrt);
strcat(temp, source+place);
strcpy(source, temp);

Crash! source is not large enough to hold the new characters.

Without the call you don't know that this is the only problem, and in
fact given the additional information the OP has now provided it was not
the only problem.

You need to allocate or reallocate space. What you need is:

char * insert(char* insrt, char* source, int place){
char *temp=NULL;

That initialisation is pointless. In fact, if you move the definition to
after nch you can...

size_t nch = strlen(insrt)+strlen(source)+1;
temp = malloc(nch);

replace the above with
char *temp = malloc(nch);

if ( temp ) {
strncpy(temp, source, place);
temp[place]='\0';
strcat(temp, insrt);
strcat(temp, source+place);
}
return temp;
}

In general, it is the sort of thing I was thinking of. Mind you, I would
not use strcat. You know where you need to copy to (with an additional
variable) so you can use strcpy and save having to scan a potentially
long string. Also I would use memcpy rather than strncpy since you know
you are not copying the entire string. Also one needs to either add
error checking or document it's lack, since if place is beyond the end
of source you have a problem.

Or you could reallocate source

void insert(char* insrt, char** source, int place){
char *temp=NULL;
size_t nch = strlen(insrt)+strlen(source)+1;
temp = realloc(*source, nch);
if ( temp ) {
strncpy(temp, source, place);
temp[place]='\0';
strcat(temp, insrt);
strcat(temp, source+place);
}
else {
/* Decide what to do if the realloc fails */
...
}
}

With the additional information now provided this would not work. We now
know that source is a pointer to a string literal (which you didn't
know when writing this) and so this would still be wrong.

This shows why the OP should have provided the complete program in the
first place.

 

[Eric Sosman]

Default User wrote On 04/17/06 15:08,:

Tony Burrows wrote:




You have two problems. One is that string literals are not modifiable.
Passing str1 to a function that modifies it is undefined behavior. For
you, luckily, that UB was a crash and alerted you to the problem.

The second problem is that even if it was modifiable, str1 is an array
of 18 characters, with no place to put in any extras.

ITYM "19 characters;" C's most characteristic error strikes
again. Also, since the O.P. is a beginner it's as well to avoid
using shorthand language that is literally untrue: str1 is not an
array, but a pointer. (And it's not a "pointer to an array,"
either.) A review of Section 6 in the FAQ <http://www.c-faq.com/>
is recommended.

 

[Default User]

Default User wrote On 04/17/06 15:08,:

ITYM "19 characters;" C's most characteristic error strikes
again.

Actually, a different error than usual. It's just not that easy to
count a longish string on screen without screwing it up. The
null-terminator was accounted for. I believe (looking at it again) I
visually dismissed the ! at the end.



Brian

 

[Fred Kleinschmidt]

I'm just learning C as another language, and I'm trying to build some
utilities into a library. I have this (crude I know) function:

void insert(char* insrt, char* source, int place){
char temp[strlen(insrt)+strlen(source)+1]; strncpy(temp, source, place);
temp[place]='\0';
strcat(temp, insrt);
strcat(temp, source+place);
strcpy(source, temp);

Crash! source is not large enough to hold the new characters.

Without the call you don't know that this is the only problem, and in fact
given the additional information the OP has now provided it was not the
only problem.

You need to allocate or reallocate space. What you need is:

char * insert(char* insrt, char* source, int place){
char *temp=NULL;

That initialisation is pointless. In fact, if you move the definition to
after nch you can...

size_t nch = strlen(insrt)+strlen(source)+1;
temp = malloc(nch);

replace the above with
char *temp = malloc(nch);

if ( temp ) {
strncpy(temp, source, place);
temp[place]='\0';
strcat(temp, insrt);
strcat(temp, source+place);
}
return temp;
}

In general, it is the sort of thing I was thinking of. Mind you, I would
not use strcat. You know where you need to copy to (with an additional
variable) so you can use strcpy and save having to scan a potentially long
string. Also I would use memcpy rather than strncpy since you know you are
not copying the entire string. Also one needs to either add error checking
or document it's lack, since if place is beyond the end of source you have
a problem.

I agree that strcat is not the best way, and I would not have used it.
I merely repeated the OP's original code.

Or you could reallocate source

void insert(char* insrt, char** source, int place){
char *temp=NULL;
size_t nch = strlen(insrt)+strlen(source)+1;
temp = realloc(*source, nch);
if ( temp ) {
strncpy(temp, source, place);
temp[place]='\0';
strcat(temp, insrt);
strcat(temp, source+place);
}
else {
/* Decide what to do if the realloc fails */
...
}
}

With the additional information now provided this would not work. We now
know that source is a pointer to a string literal (which you didn't know
when writing this) and so this would still be wrong.

The coding of the insert() method is not wrong here - it is the usage of
that code that is wrong. There's not much that can be done to prevent
a problem if the usage is incorrect.

 

[Tony Burrows]

In addition, even if string literals were modifiable, you are trying to
make the string longer, so this would not work either:
#include <stdio.h>
#include "insert.h"

Ouch! I'd missed the string size issue. The original version with the
function in the same file used a char array of length 40! When I reverted
back to that it *seems* to behave as I expected, but judging from what
you and others have said, it shouldn't since strings are immutable.

Since it would go off the end of str1.

If I was writing a function like your insert I would probably make is
malloc enough space for a new string and return the resultant string in
that rather than modifying one of the input strings.

That was my first intent, except it didn't work, but you've explained why.
I did want something like the strcpy approach though where the original
string had characters inserted.

Thanks for all the help. C is certainly different from Java!

Tony

 

[Default User]

Ouch! I'd missed the string size issue. The original version with
the function in the same file used a char array of length 40! When I
reverted back to that it seems to behave as I expected, but judging
from what you and others have said, it shouldn't since strings are
immutable.

If you had set the length, it was probably something like:

char str1[40] = "whatever";

This is NOT the same thing we were talking about. This is not a pointer
to a string literal as you had before. It's an array of char,
initialized with a string, and is modifiable.

You really need to read the FAQ.



Brian

 

[Flash Gordon]

Fred Kleinschmidt wrote:

Or you could reallocate source

void insert(char* insrt, char** source, int place){
char *temp=NULL;
size_t nch = strlen(insrt)+strlen(source)+1;
temp = realloc(*source, nch);
if ( temp ) {
strncpy(temp, source, place);
temp[place]='\0';
strcat(temp, insrt);
strcat(temp, source+place);
}
else {
/* Decide what to do if the realloc fails */
...
}
}

With the additional information now provided this would not work. We now
know that source is a pointer to a string literal (which you didn't know
when writing this) and so this would still be wrong.

The coding of the insert() method is not wrong here - it is the usage of
that code that is wrong. There's not much that can be done to prevent
a problem if the usage is incorrect.

It could be either the function for the usage that is wrong. Or both. If
the spec is that it has to work on non-malloced input objects (which is
not an unreasonable spec) then your alternative suggestion above is
wrong. If the spec for main is that it must work with the function then
the usage is wrong. If the spec does not cover this then they are
incompatible making the program as a whole wrong.

 

[Old Wolf]

void insert(char* insrt, char* source, int place){
char temp[strlen(insrt)+strlen(source)+1]; strncpy(temp, source, place);
temp[place]='\0';
strcat(temp, insrt);
strcat(temp, source+place);
strcpy(source, temp);
}

As well as what everyone else has pointed out: you should check
that 0 <= place <= strlen(source). Otherwise you will get
undefined behaviour.

 

[websnarf]

I'm just learning C as another language, and I'm trying to build some
utilities into a library. I have this (crude I know) function:

void insert(char* insrt, char* source, int place){
char temp[strlen(insrt)+strlen(source)+1];

Your compiler lets you declare that? Is this some new C99 feature? I
think gcc lets you do things like this, but I don't think its ISO C89
compliant. Basically, declaration sizes have to be compile-time
constants, in most C compilers out there. So something like:

char * temp = (char *) malloc(1+strlen (instr)+strlen (source));
if (temp) {

/* Do inner loop */

free (temp);
}

is more portable and more compliant.

strncpy(temp, source, place);
temp[place]='\0';
strcat(temp, insrt);
strcat(temp, source+place);
strcpy(source, temp);
}

Ok, you are modifying the source, but not the insrt string. I would
therefore highly recommend that you declare the function as follows:

void insert (const char* insrt, /*@out@*/char* source, int place) {

The const tells us that the object its point to will not be modified.
The /*@out@*/ declares to tools like SPLINT that the destination can be
modified by the call (personally, I don't know why SPLINT doesn't just
assume that that will happen).

As a matter of performance, I would just like to point out that you
don't need to perform any work on the characters from 0 up to but
excluding place. In other words:

strcpy (temp, insrt);
strcat (temp, source+place);
strcpy (source+place, temp);

Works just as well, without redundantly copying over the first place
characters. You could do even better if C had some sort of "copy and
give me the length" function, but it doesn't.

In a simple file, it all works, but in a static library, when I call it I
get a segnmentation fault. The villain is the final strcpy.

That's because inline strings are not modifiable -- though no C
compiler that I know of does any compile time checking of this fact for
you. I.e.,

char * s;
insert ("Constant string ", s = "Unwritable string", 11);

is wrong because "Unwritable string" cannot be modified. Changing this
to:

char s[128 /* MAGIC CONSTANT */] = "Writable string";
insert ("Constant string ", s, 9);

And the program will work once again.

I tried returning temp instead and that doesn't work either, I don't get
quite the string that exists in the function (and the compiler warns about
returning a local variable).

The storage for temp is only defined for the scope of the insert()
function. I.e., literally the part of the call stack that is used to
execute insert() disappears once it returns, and that includes the
storage for the locally scoped variables in that function.

I know there's a solution, I just have no idea what it is. Any help would
be great.

Download the Better String Library here http://bstring.sf.net

Then do the following:

#include <strdio.h>
#include "bstrlib.h"

/* ... */

bstring s;
struct tagbstring i = bsStatic ("better ");
binsert (s = bfromcstr ("a string library"), &i, 2, '?');
puts (bdatae (s, "Out of memory"));
bdestroy (s);

 

[Keith Thompson]

I'm just learning C as another language, and I'm trying to build some
utilities into a library. I have this (crude I know) function:

void insert(char* insrt, char* source, int place){
char temp[strlen(insrt)+strlen(source)+1];

Your compiler lets you declare that? Is this some new C99 feature? I
think gcc lets you do things like this, but I don't think its ISO C89
compliant. Basically, declaration sizes have to be compile-time
constants, in most C compilers out there.

Yes, variable-length arrays are a new C99 feature, and gcc implements
something very similar.

So something like:

char * temp = (char *) malloc(1+strlen (instr)+strlen (source));

Thee cast is unnecessary and generally not recommended.

See <http://www.c-faq.com/>, questions 7.7, 7.7b, 7.7c.

(The cast is required in C++. A very few people have good reasons to
compile their code as both C and C++. A few others argue that the
cast is a good idea.)

 

[Robert Gamble]

I'm just learning C as another language, and I'm trying to build some
utilities into a library. I have this (crude I know) function:

void insert(char* insrt, char* source, int place){
char temp[strlen(insrt)+strlen(source)+1];

Your compiler lets you declare that? Is this some new C99 feature?

Yes, variable length arrays are new in C99.

Robert Gamble