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

typedef char* string;

int main(void)
{
    char *names[6];
    int num_entries = 0,i=0,size=0;
    string name = (string) malloc(sizeof(char) * 16);

    printf("\nHow many names do you want to enter ? \n");
    scanf("%d",&num_entries);

    for(i=0 ; i < num_entries ; i++)
    {
        printf("\nEnter a name : ");
        gets(name);
        size = strlen(name);
        names[i] = (string) malloc(sizeof(char)*size + 1);
        strcpy(names[i],name);
    }

    for(i=0 ; i < num_entries ; i++)
        puts(names[i]);

}

in this program the string is not read the first time around the loop,however works fine for all subsequent calls,the program simply has to accept n strings,store and display them. owever it executes n-1 times.Solution?also,feel free to point any mistakes in the way pointers,allocation etc. is used,any feedback appreciated .

rootavish
  • 122
  • 12
  • 2
    `gets()` is evil. Consider using `fgets()` instead. See the BUGS section near the bottom of this page: http://manpages.debian.net/cgi-bin/man.cgi?query=fgets&apropos=0&sektion=0&manpath=Debian+7.0+wheezy&format=html&locale=en – alk Aug 17 '13 at 18:18
  • changed the gets() call to fgets(name,16,stdin) still same bug – rootavish Aug 17 '13 at 18:23
  • @alk was just making a comment for improvement, not offering an answer to the problem. That's why it's a comment, not an answer. :) Here's another comment: the preinitializers for your integers are superfluous since you always set them anyway. :) – lurker Aug 17 '13 at 18:25
  • @mbratch: 1st: Thanks for explaining my intention. 2nd: Although the preinit/s indeed aren't necessary, I'd prefer to not have uninitialised variables hanging around. This leads to my next comment for improvement: Declare variables where you need them, and this **isn't the top** of the method in most cases. – alk Aug 17 '13 at 18:29
  • No worries, alk. Sounds logical. – lurker Aug 17 '13 at 18:31
  • One more: In C do not cast the result of `malloc/calloc/realloc` as it is not necessary nor recommended. For detail please read here: http://stackoverflow.com/a/605858/694576 – alk Aug 17 '13 at 18:32
  • Another one: Always check the result of (more or less any) system call. In your case `malloc()` may return `NULL`. Dereferencing `NULL` will lead to undefined behaviuor. – alk Aug 17 '13 at 18:33

4 Answers4

2

Call gets before the loop to discard the new line left by scanf.

Or better yet, use the standard workaround to discard unread input:

int c;
while ((c = getchar()) != '\n' && c != EOF);
Anthony Accioly
  • 21,918
  • 9
  • 70
  • 118
  • `char c; while((c=getchar()!='\n')&& c!= EOF) { name[i] = c; i++; } name[i] = '\0'; ` something like this? – rootavish Aug 17 '13 at 18:29
  • 1
    @superuser47, `getchar` returns an `int` and so `int c` is what's generally used as Anthony shows. And his suggestion was just to use it to gobble up characters until the next line feed. If you want to change your code to use it as the mechanism for reading the `name`, then it would be good to show that as an edit to your original post if you have further questions on it. – lurker Aug 17 '13 at 18:33
  • superuser, you have mistaken the intent of the code. Just use it - as is - after `scanf("%d",&num_entries);` and before the loop, to discard the newline. – Anthony Accioly Aug 17 '13 at 18:33
2

The issue here, which is typical of the scanf statement, is that it does not use the newline when you entered the number of names you wanted and pressed "enter".

As a result, the newline is stuck in the stdin buffer until you do your next read, which in this case is the first name you try to read, so your first name is simply "newline". To deal with this, use getchar() to eat up the newline character so you don't have that issue anymore.

Typically, as a rule of thumb, you'll almost always want to use a getchar() or something similar after a scanf statement to deal with this issue.

I've modified your code below and works fine for me. I also cleaned it up a bit since some lines were not necessary.

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

typedef char* string;

int main(void)
{
    string names[6];
    int num_entries=0, i=0;
    string name = malloc(sizeof(char) * 16);

    printf("\nHow many names do you want to enter ? \n");
    scanf("%d",&num_entries);
    getchar();
    for(i=0 ; i < num_entries ; i++)
    {
        printf("\nEnter a name : ");
        fgets(name,16,stdin);
        names[i] = malloc(sizeof(char)*strlen(name) + 1);
        strcpy(names[i],name);
    }

    for(i=0 ; i < num_entries ; i++)
        puts(names[i]);
return 0;
}
xt454
  • 56
  • 2
  • Since you've already declared a typedef for string, you can just use `string names[6]` instead of `char *names[6]`. As for `fgets`, it'll prevent you from doing overflows on your limited string size of 16 chars. – xt454 Aug 17 '13 at 18:39
1

Here's the code with all suggestions. Note that Anthony Accioly gets credit for the answer.

int main(void)
{
    char *names[6];
    int num_entries = 0, i = 0, size = 0, c = 0;
    string name = malloc(sizeof(char) * 16);

    if ( !name )
    {
        printf( "Unable to allocate memory for name\n" );
        return(1);
    }

    printf("\nHow many names do you want to enter ? \n");
    scanf("%d",&num_entries);
    while ((c = getchar()) != '\n' && c != EOF);

    for( i = 0 ; i < num_entries; i++)
    {
        printf("\nEnter a name : ");
        gets(name);
        size = strlen(name);
        names[i] = (string) malloc(sizeof(char)*size + 1);
        strcpy(names[i],name);
    }

    for(i=0 ; i < num_entries ; i++)
        puts(names[i]);

    return(0);
}
lurker
  • 56,987
  • 9
  • 69
  • 103
0

You can also use fflush(stdin); as an alternative to getchar() or the while(...) statement.

P.S.: I am sorry for writing my suggestion here, as I do not have enough reputation to comment.

0xF1
  • 6,046
  • 2
  • 27
  • 50