tags:

views:

3977

answers:

7
+4  Q: 

Using strcat in C

Okay so I have the following Code which appends a string to another in C#, note that this is Just an example, so giving alternative string concatination methods in C# is not nessesary, this is just to simplify the example.

string Data = "";
Data +="\n\nHTTP/1.1 " + Status_code;
Data += "\nContent-Type: " + Content_Type;
Data += "\nServer: PT06";
Data += "\nContent-Length: " + Content_Lengt;
Data += "\nDate: " + Date;
Data += "\n" + HTML;

Now I'd like to do the exact same thing in C and I'm trying to do this the following way

time_t rawtime;

time ( &rawtime );

char *message = "\n\nHTTP/1.1 ";
message = strcat(message, Status_code);
message = strcat(message, "\nContent-Type: ");
message = strcat(message, Content_Type);
message = strcat(message, "\nServer: PT06");
message = strcat(message, "\nContent-Length: ");
message = strcat(message, Content_Lengt);
message = strcat(message,  "\nDate: ");
message = strcat(message, ctime(&rawtime));
message = strcat(message, "\n");
message = strcat(message, HTML);

Now, this gives me a Segment fault, I know why, I access and read on memory that i shouldn't. But the question is, how do i solve it? Could I use string.h and just do it the same way that I did in C#?

+2  A: 

message points to a char const[] that you can't write to, yet that's exactly where strcat is writing. You need to malloc() a sufficiently large buffer.

MSalters
I see, let me try this then. But are you saying that I cant use const char[] there? hm.
Filip Ekberg
No indeed. The segfault is caused by writing to the const (read-only) memory allocated for "" literals.
MSalters
Could you maybe provde a code example of how the above would be changed into that?
Filip Ekberg
+9  A: 

Change

char *message = "\n\nHTTP/1.1 ";

to

char message[1024];  
strcpy(message,"\n\nHTTP/1.1 ");

and you should be ok, up to a total message length of 1023.

Edit: (as per mjy's comment). Using strcat in this fashion is a great way of getting buffer overflows. You could readily write a small function that checks the size of the buffer and length of incoming string addition to overcome this, or use realloc on a dynamic buffer. IMO, the onus is on the programmer to check correct buffer sizes where they are used, as with sprintfs and other C strings functions. I assume that C is being used over C++ for performance reasons, and hence STL is not an option.

Edit: As per request from Filip's comment, a simple strcat implementation based on a fixed size char buffer

char buffer[MAXSIZE] = "";

int mystrcat(char *addition)
{
   if (strlen(buffer) + strlen(addition) + sizeof(char)  >= MaxSize)
     return(FAILED);
   strcat(buffer,addition);
   return(OK);
}

Using dynamic allocation;

char *buffer = NULL;

int mystrcat(char *addition)
{
   buffer = realloc(buffer, strlen(buffer) + strlen(adition) + sizeof(char));
   if (!buffer)
     return(FAIL);
   strcat(buffer, addition);
   return(OK);
}

in this case you have to free your buffer manually when you are finished with it. (Handled by destructors in C++ equivalents)

Addendum (Pax):

Okay, since you didn't actually explain why you had to create message[1024], here it is.

With char *x = "hello", the actual bytes ('h','e','l','l','o',0) (null on the end) are stored in an area of memory separate from the variables (and quite possibly read-only) and the variable x is set to point to it. After the null, there's probably something else very important. So you can't append to that at all.

With char x[1024]; strcpy(x,"hello");, you first allocate 1K om memory which is totally dedicated to x. Then you copy "hello" into it, and still leave quite a bit of space at the end for appending more strings. You won't get into trouble until you append more than the 1K-odd allowed.

End addendum (Pax):

Shane MacLaughlin
that's a terrible recipe for enabling buffer overflow exploits ...
mjy
Yup, thats why we use stl strings, MFC CStrings, or various other smart strings in C++ these days.
Shane MacLaughlin
However this is C and not C++. And the example didn't really run as I hoped. Do I need to change all code to to strcpy or do i still use strcat? Still using strcat gives a lot of other errors.
Filip Ekberg
@Filip, it depends on the context. Preallocated fixed size buffers are faster than dynamic allocation, and less likely to fail due to resource issues. Are you after memory and speed efficiency or a reliable general purpose sub-routine.
Shane MacLaughlin
I just want a simple, readable concatenation of the above strings.I have a methods which takes 4 char pointers and those are supposed to be concatinated into a single string which i then return a pointer too. So speed, effieciency or anything else is less important,understanding is what is important
Filip Ekberg
This answer is fine if you can be certain that the total message length won't exceed 1023 characters (i.e. are you in control of Status_code etc.?). strcat is fine; strcpy would overwrite your string rather than append it. It's certainly more readable than boundary-checking if you don't have to.
Paul Stephenson
...that is, you should take the strcpy in @smacl's code above first, as that initialises the string. But don't change all your existing strcats to strcpys.
Paul Stephenson
Oh, and you don't need to say 'message =' all the time, as strcat just returns the message pointer you had already. That would make it even more readable.
Paul Stephenson
You don't need to worry about buffer overflow if you control the strings being appended, and you frequently do. I added some text to explain it further, then +1.
paxdiablo
This was very much helpfull. Thank you again!
Filip Ekberg
This is a Shlemiel the painter's algorithm - read http://www.joelonsoftware.com/articles/fog0000000319.html for an explanation of and a solution to the problem
Christoph
@Pax, thanks for the addition. I think this horse has now been thoroughly flogged and be prepared for the stew ;)
Shane MacLaughlin
Seriously, people - don't do this. Use a function of the sprintf-family and save yourself some headaches!
Christoph
@Christoph, how about providing an example for that argument?
Filip Ekberg
@Filip: It's basically the same argument for using a StringBuilder and not += in C#. The issue in C is worse, because you get quadratic behaviour because of the repeated calls to strlen(). I already linked to a blog post explaining this and posted solutions using both snprintf and asprintf.
Christoph
Im reading it now. Didnt see that it was You whom pasted that link. Thanks :)
Filip Ekberg
@Christoph, asprintf doesn't appear to be implemented as yet in the MS compilers, although I would certainly agree that an sprintf derivation is the way to go if you have all your arguments available in advance. If not you are stuck with the above.
Shane MacLaughlin
@smacl: But Microsoft's C lib provides _vscprintf(): http://msdn.microsoft.com/en-us/library/w05tbk72(VS.71).aspx - you can easily implement your own asprintf() function in a few lines of code...
Christoph
@Christoph - Thanks for that, you learn something new every day.
Shane MacLaughlin
+6  A: 

Start from using the safer strncat function. In general always use the safer 'n' functions that will not overflow if the size of a string is bigger than a specific size.

In C you need to take care of string sizes yourself. So you need to know how big the resulting string will be and accommodate for it. If you know the sizes of all the strings at the left side, you should create a buffer big enough to hold the resulting string.

kgiannakakis
+1  A: 

As said previously, you have to write to a sufficiently large buffer. Unfortunately, doing so is a lot of extra work. Most C applications that deal with strings use a dynamically resizable string buffer for doing concatenations.

glib includes an implementation of this, glib strings, which I recommend using for any application that uses strings heavily. It makes managing the memory easier, and prevents buffer overflows.

Jeff M
A: 

The safe way to do this in classic C style is:

 char *strconcat(char *s1, char *s2)
 {
    size_t old_size;
    char *t;

    old_size = strlen(s1);

    /* cannot use realloc() on initial const char* */
    t = malloc(old_size + strlen(s2) + 1);
    strcpy(t, s1);
    strcpy(t + old_size, s2);
    return t;
  }

  ...

  char *message = "\n\nHTTP/1.1 ";
  message = strconcat (message, Status_code);
  message = strconcat (message, "\nContent-Type: ");

Now you can say a lot of bad things about it: it's inefficient, it fragments your memory, it's ugly ... but it's more or less what any language with a string concatenation operator and C type (zero-terminated) strings will do (except that most of these languages will have garbage collection built-in).

mjy
-1: This is a massive memory leak. You need to free the s1 pointer if and only if it wasn't malloc-ed by you in the first place (which makes the code more horrible if you have a const char * starting value).
Paul Stephenson
That had better be a static function.
Cirno de Bergerac
Yes, it leaks memory and that could easily be avoided with realloc() and copying the initial const char*. All simple C solutions are memory inefficient, for example, the constant strings are all duplicated (even with realloc). Also, you always need to free the final result explicitly.
mjy
Those things are outside the scope of the original answer though. If the OP wanted a memory efficient version as well, the answer would be much longer. My focus was on something short and equivalent to the C# code that doesn't use an unsafe static buffer or non-libc-libraries.
mjy
+3  A: 

I wonder why no one mentioned snprintf() from stdio.h yet. That's the C way to output multiple values and you won't even have to convert your primitives to strings beforehand.

The following example uses a stack allocated fixed-sized buffer. Otherwise, you have to malloc() the buffer (and store its size), which would make it possible to realloc() on overflow...

char buffer[1024];
int len = snprintf(buffer, sizeof(buffer), "%s %i", "a string", 5);
if(len < 0 || len >= sizeof(buffer))
{
    // buffer too small or error
}

Edit: You might also consider using the asprintf() function. It's a widely available GNU extension and part of TR 24731-2 (which means it might make it into the next C standard). The example from above would read

char * buffer;
if(asprintf(&buffer, "%s %i", "a string", 5) < 0)
{
    // (allocation?) error
}

Remember to free() the buffer when done using it!

Christoph
+1  A: 

Have not seen any mention of the strlcpy, strlcat function, which is similar to the 'n' functions except also takes into account the trailing 0. Both take a third argument indicating the maximum length of the output buffer and are found in string.h.

example:

char blah[]="blah";
char buffer[1024];
strlcpy(buffer,"herro!!!",sizeof(buffer));
strlcat(buffer,blah,sizeof(buffer));
printf("%s\n",buffer);

Will output "herro!!!blah"

char blah[]="blah";
char buffer[10];
strlcpy(buffer,"herro!!!",sizeof(buffer));
strlcat(buffer,blah,sizeof(buffer));
printf("%s\n",buffer);

will output "herro!!!b" due to the limited size of buffer[], with no segfaulting. ^^

Only problem is not all platforms seem to include it in their libc (such as linux ._.), most all BSD varients DO seem to have it.

In that case, code for both functions can be found here and easily added: strlcpy, strlcat, the rest of string.h

erisu
strl* is less widely available, and ends up being less portable. At least strn* is standard afaik.
Calyth