0

I've resumed C coding for fun after a several year absence.

I've given myself an exercise to safely copy text from standard input to strings using fgets(), and copy to a string just big enough, i.e. only with enough capacity to hold the no. of chars I've actually typed, ultimately to make lists, stacks etc. from scratch, in other words, playing with pointers.

The only way I've managed this smells of kludge to me, as I'm defining the destination string variable for strcpy() late in the control flow. Is there a more elegant/dynamic way to do this?

#inlcude <stdio.h>
#include <string.h>

#define MAXLENGTH 20

int main(int argc, char *argv[]) {

    char message[MAXLENGTH];

    printf("Enter a string: \n");
    fgets(message, MAXLENGTH, stdin);

    /* various tests here, omitted for brevity */

    char destinationString[strlen(message)];

    /*
     *    Just testing to prove that
     *    the strlen() of the destination
     *    string is LESS than MAXLENGTH
     */
    printf("Here's the strlen() of destinationString: %lu\n", strlen(destinationString));
    printf("Here's the sizeof() destinationString: %lu,\n" sizeof(destinationString));
    printf("Here's the contents of the copy: %s", destinationString);

    return 0;
}
Tharif
  • 13,794
  • 9
  • 55
  • 77
  • 2
    You likely have a buffer overflow in `destinationString` (I'm assuming you have a `str[n]cpy` somewhere before the `printf` statements). You need an extra character to store the last termination character. – tangrs Jul 17 '15 at 04:40
  • @tangrs: I agree, though it could be OK if the code checks for a newline character and eliminates it. However, if the line is longer than 18 characters (which is not very long), there won't be a newline in the buffer, and then the overflow probably will occur. – Jonathan Leffler Jul 17 '15 at 04:42
  • It is sometime since I wrote code in C. In earlier C versions, you can declare variables only at the beginning of the function. Your approach looks pragmatic to me and also declaring a variable just before its use reduces the scope of that variable and IMO it is a good thing. – Dakshinamurthy Karra Jul 17 '15 at 04:49
  • destinationString is a variable length array, VLA, which is almost certainly inappropriate for your needs. – David Heffernan Jul 17 '15 at 05:03
  • On UNIX-like systems, the function `strdup()` exists for this kind of task. – fuz Jul 17 '15 at 06:45
  • Funnily enough, @tangrs I didn't get any buffer overflow in the original code, tho' I did deliberately choose four or five letter words to test. strcpy( ) successfully copied all the "message[ ]" chars including "\n". – gregreedee Jul 20 '15 at 11:37
  • A buffer overflow will cause undefined behaviour. It's quite likely you had one but it worked anyway by chance. – tangrs Jul 20 '15 at 13:16

2 Answers2

3

You can certainly do this dynamically by using malloc.

Consider something like this:

int main(int argc, char *argv[]) {
    char *destinationString;
    /* ... */

    /* Don't forget to allocate one extra byte for the termination character */
    destinationString = malloc(strlen(message) + 1);

    if (!destinationString)
        return -1;

    strcpy(destinationString, message);
    /* Note: Normally, you should probably use strncpy to avoid overflow
       but here, we're sure that there's enough space so strcpy is acceptable */

    /* ... */

    free(destinationString); /* When you're done using it */

    /* ... */
}

I also pointed this out in the comments but to re-iterate, you actually need to allocate strlen(message) + 1 bytes in your destination string buffer or else it will overflow. The extra character is to store the null termination character at the end of C strings.

tangrs
  • 9,709
  • 1
  • 38
  • 53
0

Code has a number of choices. Here are 2:

  1. malloc() and later free() right sized memory similarly answered by @tangrs. Note that sizeof() destinationString will be the size of a pointer.

    size_t size = strlen(message) + 1;
    char *destinationString = malloc(size);
    memcpy(destinationString, message, size);
    
  2. Use variable length array, VLA, available in C99 and optionally in C11.

VLA approach with code clean-up

#include <string.h>
#define MAXLENGTH 20

int main(int argc, char *argv[]) {
    char message[MAXLENGTH];

    printf("Enter a string: \n");
    if (fgets(message, sizeof message, stdin) == NULL) { 
      return -1;
    }

    // Use type `size_t`
    size_t size = strlen(message) + 1;
    char destinationString[size];
    memcpy(destinationString, message, size);

    // Notice "%zu"
    // `sizeof destinationString` is the size of an array
    printf("Here's the strlen() of destinationString: %zu\n", strlen(destinationString));
    printf("Here's the sizeof() destinationString: %zu,\n" sizeof destinationString);
    printf("Here's the contents of the copy: \"%s\"", destinationString);
    return 0;
}

Input "Hello!" Enter

Here's the strlen() of destinationString: 8
Here's the sizeof() destinationString: 9,
Here's the contents of the copy: "Hello!
"

On my system the inputs ended with a "\r\n". To rid the buffer of those potential pesky characters, use:

fgets(message, sizeof message, stdin);
buffer[strcspn(message, "\r\n")] = '\0';
size_t size = strlen(message) + 1;
...
Community
  • 1
  • 1
chux - Reinstate Monica
  • 143,097
  • 13
  • 135
  • 256
  • Thanks so much! How did you define the size_t type? I don't see any definitions on top of the modified code ... – gregreedee Jul 20 '15 at 11:33
  • @gregreedee `size_t` is defined in `` (and other headers). It is some unsigned integer type with a range of 0 to `SIZE_MAX`. `SIZE_MAX` is _at least_ 65535. – chux - Reinstate Monica Jul 20 '15 at 14:22
  • Thanks again, now I know about size_t. and more appropriate printf( ) formatting! I ran your version, it worked! My research told me I could get away with using stdlib.h as it quotes stddef.h . Re the buffer[ ] ?char array/string in your stripping for "\r\n" characters, in what context is it used? Wouldn't fgets( ) be going straight into this buffer, instead of message[ ]? – gregreedee Jul 23 '15 at 22:36
  • Typical code after a `fget()` wants to rid the buffer of end-of-line characters. This is is typically `"\n"` , but other OS's use `"\r\n" or "\r". `buffer[strcspn(message, "\r\n")] = '\0';` nicely rids these _potential_ characters. This make s code more portable. – chux - Reinstate Monica Jul 27 '15 at 02:07
  • It works, I implemented it thus: message[strcspn(message, "\r\n")] = '\0'; You're a legend. Thanks. – gregreedee Aug 03 '15 at 12:28