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: Output ]

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

You are returning an array that is on the stack. When the function ends so does that part of the stack. The return from these functions point to stuff that can and does change — Ed Heal
– Ed Heal, Commented Feb 21, 2016 at 9:09
I have in-lined the image for you, but as Ed says, the output is plain-text and could have more easily and readably been posted as such. — Clifford
– Clifford, Commented Feb 21, 2016 at 9:16

chqrlie · Accepted Answer · 2016-02-21 09:56:51Z

2

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.

edited Feb 21, 2016 at 9:56

answered Feb 21, 2016 at 9:44

chqrlie

153k12 gold badges146 silver badges232 bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

Ed Heal Over a year ago

Is this a bit of hassle when the maximum length is knoiwn

chqrlie Over a year ago

@EdHeal: of course if the maximum length is known, this function should take a pointer to a local array in the callers scope and its size. For the OP's API, the above code works, and could be simplified if aprintf is available in the target environment.

Rabbid76 · Accepted Answer · 2018-09-06 16:04:37Z

1

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()?

edited Sep 6, 2018 at 16:04

answered Feb 21, 2016 at 9:26

Rabbid76

212k30 gold badges164 silver badges206 bronze badges

4 Comments

Ed Heal Over a year ago

I think you need to add fflush after the printfs

chqrlie Over a year ago

This does not work: you must strip the '\n at the end of the line read by fgets(), plus you must check for end of file.

M.M Over a year ago

gets should not be used in an answer

Thomas Padron-McCarthy Over a year ago

This explains nothing. You just did his homework for him.

Jens · Accepted Answer · 2016-02-21 09:17:35Z

1

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.

edited Feb 21, 2016 at 9:17

answered Feb 21, 2016 at 9:10

Jens

73.3k16 gold badges134 silver badges186 bronze badges

2 Comments

Ed Heal Over a year ago

.. That would not help much either - see my comment above

Muhammad Hashim Shafiq Over a year ago

I have done this way but it always diplay nickname 3 times as output.

Ed Heal · Accepted Answer · 2016-02-21 10:03:47Z

1

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);

edited Feb 21, 2016 at 10:03

answered Feb 21, 2016 at 9:24

Ed Heal

60.3k18 gold badges91 silver badges137 bronze badges

1 Comment

chqrlie Over a year ago

Note that because of the trailing \n, the buffer should be defined with an extra byte: its size should be the maximum length of the field + 2.

Thomas Padron-McCarthy · Accepted Answer · 2016-02-21 09:41:00Z

0

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.

edited Feb 21, 2016 at 9:41

answered Feb 21, 2016 at 9:09

Thomas Padron-McCarthy

27.7k8 gold badges55 silver badges77 bronze badges

Comments

Kérdezösködő Indián · Accepted Answer · 2016-02-21 10:09:10Z

0

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

answered Feb 21, 2016 at 10:09

Kérdezösködő Indián

6821 gold badge8 silver badges20 bronze badges

Collectives™ on Stack Overflow

sprintf in C giving wrong output

6 Answers 6

2 Comments

4 Comments

2 Comments

1 Comment

Comments

Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

6 Answers 6

2 Comments

4 Comments

2 Comments

1 Comment

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related