4
\$\begingroup\$

Seems to work fine, I guess it won't work if the destination array's size is not defined by a macro, but a variable or something else, I thought about adding a third argument which is the size and let the user enter it, but that might be worse.

#include <stdio.h> #include <stdlib.h> #include <string.h> #define MAX_SIZE 100 char *ptr_str_cat(char *dest, const char* src){ char *original; original = dest; if((strlen(dest) + strlen(src)) > MAX_SIZE){ puts("The source string is too big for the destination"); exit(EXIT_FAILURE); } dest = strchr(dest, '\0'); *dest = '0'; while((*dest++ = *src++) != '\0'){ ; /*do nothing*/ } return original; } int main(void){ char dest [MAX_SIZE]; char source [MAX_SIZE]; puts("Enter the destination string"); fgets(dest, sizeof(dest), stdin); dest[strlen(dest) -1 ] = '\0'; /*Remove trailing newline*/ puts("Enter the source string"); fgets(source, sizeof(source), stdin); printf("%s", ptr_str_cat(dest, source)); return 0; }
\$\endgroup\$
3
  • \$\begingroup\$ "but that might be worse" Could you elaborate about your concerns? IMO adding a 3rd parameter to specify the maximum destination size would make your function way more reusable. Also the line *dest = '0'; seems to be superfluous. \$\endgroup\$ Commented Apr 3, 2017 at 17:21
  • 3
    \$\begingroup\$ Is there a non-pointer version of strcat()? Was the goal here to replicate strcat() or to write a better version of it? \$\endgroup\$ Commented Apr 3, 2017 at 17:25
  • \$\begingroup\$ Neither, I suppose. I don't know the standard implementation so I can't tell how close mine is to the standard. \$\endgroup\$ Commented Apr 3, 2017 at 18:55

2 Answers 2

Reset to default
1
\$\begingroup\$

Unclear goal

I'm not sure what your goal was here, since your function kind of does the same thing as the library function strcat(), except for the buffer size check. But your buffer size check is awkward, because how are the callers supposed to know what the max buffer size is? What if someone calls your function with a smaller buffer than that? Or for that matter, what if they had a bigger buffer but your function refused to work on that sized buffer?

If you really want to have a buffer size check, I'd recommend passing in the buffer size as an extra argument:

char *my_strcat(char *dest, size_t destMax, const char *src);

Off by one bug

Your check on exceeding the maximum buffer size is off by one. Currently, if you have two strings of length 50, you allow the strings to be concatenated, but you will end up creating a new string of length 100 plus an extra null terminating character, for a total of 101 bytes, which overflows your buffer. You could fix this by changing the > in your check to >=.

Extra work

Right now, you call strlen(dest) once to find the length of dest, and then you call strchr(dest, '\0') to find the length a second time. If you just remembered the length from the first call, you could eliminate the call to strchr():

size_t destLen = strlen(dest); // ... dest += destLen;

Better copy

Similar to the above, if you remembered the length of the source string, you could use it to do the copy via memcpy(), which should be faster than your own copy loop:

size_t srcLen = strlen(src); // ... memcpy(dest, src, srcLen+1);

Rewrite

Here is how I would have rewritten your function. Notice also that I removed the variable original by using dest + destLen in the call to memcpy():

char *ptr_str_cat(char *dest, size_t destMax, const char* src) { size_t destLen = strlen(dest); size_t srcLen = strlen(src); if (destLen + srcLen >= destMax) { puts("The source string is too big for the destination"); exit(EXIT_FAILURE); } memcpy(dest + destLen, src, srcLen + 1); return dest; }
\$\endgroup\$
4
\$\begingroup\$

ptr_str_cat function:

  • The ISO C90 strcat requires the string dest to have sufficient space to hold the result of the concatenation. On the other hand strncat takes another argument size_t n and won't copy more then n characters to dest (+ the null terminator). You should definitely consider to implement one of these approaches, since the macro seems rather randomly chosen (you don't know whether dest is big enough to hold src).

    Edit: See @JS1's answer to take a look at the approach of strncat. He also has a good catch with the Off-by-one-error.

  • If you want to completely leave out library function calls, you could even replace the strchr call with a simple loop. 

    while(*dest != '\0') dest++;

  • There is no need to explicitly overwrite the null terminator in dest. It will get overwritten by the contents of src and in the case of an empty src the null terminator is actually wanted.

  • The while-loop doesn't require extra curly braces. The NULL statement is sufficient.

    while((*dest++ = *src++) != '\0') ; /* Intentionally left blank */

  • This is what you would come up with:

    char *ptr_str_cat(char *dest, const char* src){ char *original = dest while(*dest != '\0') dest++; while((*dest++ = *src++) != '\0') ; /* Intentionally left blank */ return original; }

    You don't even need to include <stdlib.h> using this implementation (since you don't use exit anymore).

main function:

  • Minor: dest[strlen(dest) - 1] = '\0'; will overwrite valid input (not the '\n'), when the first input line is longer than MAX_SIZE.

EDIT:

Good question regarding how to handle large input. I think this somehow depends on the target. I'd say our goal here is to create a piece of software, which is able to read in two strings (of any unknown size, separated by '\n') and later on have a pointer to the concatenation of these two strings.

One approach would be to allocate MAX_SIZE bytes of memory using malloc. Using fgets we fill the allocated memory and each time the string does not end on '\n' we reallocate MAX_SIZE more bytes and read in more characters. This will work until you run out of memory.

\$\endgroup\$
2
  • \$\begingroup\$ I always add braces to everything in case I need to edit it in the future and avoid forgetting them. What would be the best way to handle input larger than the max size? Check if the last character is '\n', and if it's not, unget the character, read it into the array, and recall ptr_str_cat? \$\endgroup\$ Commented Apr 3, 2017 at 18:58
  • \$\begingroup\$ @MichaelX I edited my post to answer your question. \$\endgroup\$ Commented Apr 3, 2017 at 21:41

You must log in to answer this question.