sprintf in C giving wrong output

Question

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

#define MAX_STRING  20

char *getFirstName() {
    char firstName[MAX_STRING];
    printf("Please enter your first name: ");
    gets(firstName);
    return (firstName);
}

char *getLastName() {
    char lastName[MAX_STRING];

    printf("Please enter your last name: ");
    gets(lastName);
    return (lastName);
}

char *getNickName() {
    char nickName[MAX_STRING];
    printf("Please enter your nick name: ");
    gets(nickName);
    return (nickName);
}

char *getCompleteName(const char *firstName,
                      const char *lastName,
                      const char *nickName) {
    char *completeName;
    sprintf(completeName, "%s \"%s\" %s", firstName, nickName, lastName);
    return (completeName);
}

int main() {
    char *firstName;
    char *lastName;
    char *nickName;
    char *completeName;

    firstName   = getFirstName();
    lastName    = getLastName();
    nickName    = getNickName();

    completeName = getCompleteName(firstName, lastName, nickName);
    printf("Hello %s.\n", completeName);
    return (EXIT_SUCCESS);
}

What is wrong with the code. It always print nickname in all the the three varaiables firstName, lastName and nickName.

Output: ]

it must display the complete name. I think error is in the getCompleteName function.

Answer 1

In function getCompleteName, you must allocate memory for completeName before you compose the contents into it:

char *getCompleteName(const char *firstName, 
                      const char *lastName,
                      const char *nickName)
{
    size_t size = strlen(firstName) + strlen(lastName) + strlen(nickName) + 5;
    char *completeName = malloc(size);
    if (completeName) {
        snprintf(completeName, size, "%s \"%s\" %s", firstName, nickName, lastName);
    }
    return completeName;
}

Notes:

• the calling function will be responsible for freeing the memory. This allocation scheme is practical but error prone, known to cause memory leaks in sloppy implementations.
• the rest of the code has the same problem: pointers must point to allocated memory, static arrays or local arrays in the calling function.
• you must not use gets(), it cannot be used safely. Use fgets() and strip the trailing linefeed.

Answer 2

char* completeName;
sprintf(completeName,"%s \"%s\" %s",firstName,nickName,lastName);

Where does completeName point to? This invokes undefined behavior due to use of an indeterminate value.

You should provide static storage, e.g. with

static char completeName[128];

The next problem is that automatic variables, such as go out of scope when the function returns. You should make the arrays static to avoid this.

Answer 3

1) You cannot use the array on the stack - perhaps this could would be better

 void getDetail(const char * const prompt, char *detail, int maxSize)
 {
    printf("%s:", prompt);
    fflush(stdout); // Gives the user a chance to set it
    if (fgets(detail, maxSize, stdin) == NULL) {
       detail[0] = 0; // EOF - Empty string
    }
    else
    {
       // Strip off new line
       size_t l = strlen(detail);
       if (detail[l - 1] == '\n') detail[l - 1] = 0;
    }

 }

2) Using it

char firstName[MAX_SIZE];
getDetail("Please enter you first name", firstName, MAX_SIZE);


... ditto for the others

3) Now making the complete name

char completeName{MAX_SIZE * 3 + 10]; // Cannot be bothered to work out the exta but that will be enough
sprintf(completeName,"%s \"%s\" %s",firstName,nickName,lastName);

... If you wish put this into a function passing in the array to it. But does not seem worthwhile in the example

4) Printing the complete name

printf("Complete name is :%s\n", completeName);

Answer 4

I recommend to use arrays of char and pass them to your function as parameter. You local array char firstName[MAX_STRING]; goes out of scope if getFirstName terminates. This meas the variable is not longer accessible after getFirstName has terminated and a pointer to the variable is undefined behaivoir.

#include <stdio.h>

#define MAX_STRING 10

void getFirstName( char *firstName )
{
    printf("Please enter your first name: ");
    fflush( stdout );
    fgets( firstName, MAX_STRING, stdin );
}

void getLastName( char *lastName )
{
  printf("Please enter your last name: ");
  fflush( stdout );
  fgets( lastName, MAX_STRING, stdin );
}

void getNickName( char *nickName )
{
    printf("Please enter your nick name: ");
    fflush( stdout );
    fgets( nickName, MAX_STRING, stdin );
}

void getCompleteName (
         char*          completeName,
         const char*    firstName,
         const char*    lastName,
         const char*    nickName)
{
  sprintf(completeName,"%s \"%s\" %s",firstName,nickName,lastName);
}

int main ()
{
  char firstName[MAX_STRING];
  char lastName[MAX_STRING];
  char nickName[MAX_STRING];
  char completeName[MAX_STRING*3+10];

  getFirstName( firstName  );
  getLastName( lastName );
  getNickName( nickName );

  getCompleteName( completeName, firstName, lastName, nickName );
  printf("Hello %s.\n",completeName);
  return(EXIT_SUCCESS);
} 

Apart form this use fgets insted of gets, because fgets checks the maximum number of characters to be read (including the final null-character) See How to read from stdin with fgets()?

Answer 5

The pointer variable completeName is uninitialized, causing what the C standard calls unidentified behaviour. Anything can happen.

You are also trying to return arrays from your functions. They are converted to pointers to the first element in each array, and that pointer is returned. But since the array is a local variable, with what is called "storage class auto", it disappears when the function returns. Rhe memory may be re-used for other things, but the pointer still points to that place in memory. Again, the behaviour is undefined.

Answer 6

Use arrays of char and fgets instead of gets. By the way, your initial code produces segmentation error on Ubuntu 15.10 and gcc (Ubuntu 5.2.1-22ubuntu2) 5.2.1 20151010.

SK