2

The question states that I take input from the user to enter n number of names using structures in C. I have written the following code:

#include<stdio.h>
typedef struct employee{ 
    int code; 
    char name[]; 
} emp; 

int main () {
    emp em; 
    int n; 
    printf ("Number of entries: "); 
    scanf ("%d", &n); 
    
    int i; 
    for (i=0;i<n;i++){ 
        printf ("Enter name: "); 
        gets(em[i].name); 
    }

    return 0; 
}

I am getting an error in this code:

T2.c:16:16: error: subscripted value is neither array nor pointer nor vector
         gets(em[i].name);

How do I write the correct code??

I even tried putting dimensions in both the arrays used and using scanf function. Nothing seems to be working and I am getting errors again and again,

Andreas Wenzel
  • 22,760
  • 4
  • 24
  • 39
Arnav Garg
  • 21
  • 2
  • 1
    Why is it in a loop at all? You have a single "employee". Your code has many other issues though – Eugene Sh. May 18 '23 at 13:44
  • 1
    [Don't use `gets`](https://stackoverflow.com/questions/1694036/why-is-the-gets-function-so-dangerous-that-it-should-not-be-used). `emp em;` defines a single instance. Use `emp em[n];` after n is determined. – Jason May 18 '23 at 13:48
  • @EugeneSh. I would really appreciate if you could point out the issues – Arnav Garg May 18 '23 at 13:50
  • @Jason When I put the dimension of the struct, this is what I get: Number of entries: 3 Enter name: Enter name: I don't know why the output is broken. – Arnav Garg May 18 '23 at 13:51
  • That is due to `scanf` shenanigans. It doesn't play nicely with... anything. What is happening is it read the number, and then left the `'\n'` in the input buffer. I personally never learned `scanf` because it is so temperamental (I'm a professional by the way). [This post](https://sekrit.de/webdocs/c/beginners-guide-away-from-scanf.html) has some good information on using `scanf` properly and also how to avoid it all together. – Jason May 18 '23 at 13:55
  • 1
    @Jason: Your suggestion to use `emp em[n];` requires that the compiler supports [variable-length arrays](https://en.wikipedia.org/wiki/Variable-length_array), which is not required by the ISO C standard. Some compilers actually do not support them, for example the Microsoft Compiler does not. – Andreas Wenzel May 18 '23 at 13:55
  • @AndreasWenzel True. I personally avoid them, but it looks like it worked for him. For a toy like this, I don't see a problem though. He has bigger problems with that 0-length `char` array in the struct =] I'll just make an answer lol. – Jason May 18 '23 at 13:58

2 Answers2

2

There are a handful of issues here. First, your struct as defined is the size of a single int:

typedef struct employee{ 
    int code; 
    char name[]; 
} emp; 

That name parameter has 0 size. This kind of definition is useful in more advanced methods of allocation which I'm guesssing is a bit beyond your skill for now. We need to provide an actual size there:

#define MAX_NAME_LEN 128

typedef struct employee{
    int code;
    char name[MAX_NAME_LEN];
} emp;

The original error in your code is due to trying to access the defined struct as if it is an array, when it is a single instance:

emp em;

The method I suggested in the comments was to just use n for this:

emp em[n];

This is okay if your compiler supports it, however, this is a special kind of array called a VLA. If we want to avoid that, we can do the same as we did for the name parameter and provide a size:

#define MAX_STUDENTS 32
int main() {
   emp em[MAX_STUDENTS];

Personally, I don't even know how to use scanf as it is pretty useless outside of homework assignments like this. I substituted your use of scanf and gets for fgets which is the safe older brother of gets. Don't use gets. Note that atoi is a simple string to int conversion function. Since we just defined a max size for the students array, we need to enforce that limit so we don't read outside the bounds of that array:

    int n = atoi(line);
    if (n <= 0 || n > MAX_STUDENTS) {
        printf("student count %d out of range (0, %d]\n", n, MAX_STUDENTS);
        return 1;
    }

Here is a final solution that should get you started:

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

#define MAX_NAME_LEN 128
#define MAX_STUDENTS 32

typedef struct employee{
    int code;
    char name[MAX_NAME_LEN];
} emp;

int main () {
    char line[MAX_NAME_LEN];
    emp em[MAX_STUDENTS];
    printf ("Number of entries: ");
    fgets(line, MAX_NAME_LEN, stdin);

    // fgets keeps newline, so remove it.
    line[strcspn(line, "\n")] = '\0';
    int n = atoi(line);
    if (n <= 0 || n > MAX_STUDENTS) {
        printf("student count %d out of range (0, %d]\n", n, MAX_STUDENTS);
        return 1;
    }

    int i;
    for (i=0;i<n;i++){
        printf ("Enter name: ");
        fgets(em[i].name, MAX_NAME_LEN, stdin);
        em[i].name[strcspn(em[i].name, "\n")] = '\0';
    }

    return 0;
}

Please note that a more generic solution to this problem that does not include using these MAX_* constants would use dynamic memory allocation via malloc or calloc. That kind of solution would allow a number of students and the length of their names to only be bounded by the amount of memory available on your system. However, guessing by the assignment, you haven't gotten to that subject yet.

Jason
  • 2,493
  • 2
  • 27
  • 27
  • I'd recommend giving `scanf` a second look. The problem with line based input is (1) it is line based, so it makes stricter than necessary assumptions on the data format; (2) you *still* need to parse the input which is typically non-trivial, especially when it must be robust. For example, to quote the `atoi` man page: "The behavior is the same as strtol(nptr, NULL, 10); **except that atoi() does not detect errors.**" (Emphasis by me.) Undetected errors are a major reason programs don't work as expected. `scanf` does a lot of nice work for parsing and error detection. – Peter - Reinstate Monica May 18 '23 at 14:28
  • @Peter-ReinstateMonica Yeah, `scanf` is fine for simple homework like this, but I personally could not provide a good answer with it. I used the same logic to use `atoi` since that is less robust and simple for this kind of assignment. Returning 0 on error is good enough here as 0 is not a proper input for this specific problem. Proper error detection with `strtol` is not trivial either. Please feel free to edit my answer with a proper `scanf` usage. – Jason May 18 '23 at 14:33
1

In the loop

for (i=0;i<n;i++){ 
    printf ("Enter name: "); 
    gets(em[i].name); 
}

you appear to be using em as if it were an array of n elements. However, it is not an array. It is only a single element.

To fix this, you could define em as a variable-length array, like this:

int main( void )
{
    int n;
    printf("Number of entries: ");
    scanf("%d", &n);
    emp em[n];

    [...]

However, not all compilers support variable-length arrays, and compilers are not required to support them by the ISO C standard.

Alternatively, you could define a fixed-length array, like this:

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

#define MAX_ENTRIES 20

[...]

int main( void )
{
    emp em[MAX_ENTRIES];
    int n;
    printf("Number of entries: ");
    scanf("%d", &n);

    //verify that number of entries is not so high that it would
    //overflow the buffer
    if ( n > MAX_ENTRIES )
    {
        printf( "Too many entries!\n" );
        exit( EXIT_FAILURE );
    }

    [...]

This solution will work on all compilers, as it does not require that the compiler supports variable-length arrays.

Another issue is that in the line

gets(em[i].name); 

it is your responsibility to ensure that there is sufficient memory to store the string. In your program, there is not sufficient memory to store a string of any length, because you are not even providing sufficient room for the terminating null character of the string.

In the declaration

typedef struct employee{ 
    int code; 
    char name[]; 
} emp; 

the member name is a flexible array member. If you decide to use a flexible array member, then it is your responsiblity to ensure that there is additional memory available immediately after the object, to store the data for the flexible array member. In your program, you are not doing this. It probably was not your intention to use a flexible array member in the first place, so I will not explain any further how to use one.

In order to ensure that the name has sufficent space for storing a string, you could specify a fixed number of characters that are reserved for storing the string, for example like this:

typedef struct employee {
    int code;
    char name[100];
} emp;

However, if the user enters more than 100 characters, then your program will overflow the array name, which will invoke undefined behavior. This means that your program may crash or misbehave in some other way.

To prevent this, I recommend that you use the function fgets instead of gets. Note, however, that in contrast to gets, fgets will write the newline character into the string, so you will probably want to remove it.

Yet another issue is that the line

scanf ("%d", &n);

will leave the newline character on the input stream, which will cause the next call to gets/fgets to read an empty line, which is not what you want. Therefore, you must discard this newline character (and any other remaining characters on that line), for example like this:

int c;

do
{
    c = getchar();

} while ( c != '\n' && c != EOF );

Here is the entire program, which does not use variable-length arrays:

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

#define MAX_ENTRIES 20

typedef struct employee
{
    int code;
    char name[100];
} emp;

int main( void )
{
    emp em[MAX_ENTRIES];
    int n;
    int c;

    //read number of entries from user
    printf( "Number of entries: " );
    scanf( "%d", &n );

    //discard remainder of line
    do
    {
        c = getchar();

    } while ( c != '\n' && c != EOF );

    //verify that number of entries is not so high that it would
    //overflow the buffer
    if ( n > MAX_ENTRIES )
    {
        printf( "Too many entries!\n" );
        exit( EXIT_FAILURE );
    }

    //read individual entries from user
    for ( int i=0; i < n; i++ )
    {
        char *p;

        printf( "Enter name: " );
        fgets( em[i].name, sizeof em[i].name, stdin );

        //attempt to find the newline character
        p = strchr( em[i].name, '\n' );
        if ( p == NULL )
        {
            //since we found no newline character, we must
            //assume that the line was too long to fit into
            //the memory buffer
            printf( "Line was too long to fit into the buffer!\n" );
            exit( EXIT_FAILURE );
        }

        //remove the newline character by overwriting it with a
        //null character
        *p = '\0';
    }

    //print back the stored strings
    printf( "\nStored the following strings:\n" );
    for ( int i=0; i < n; i++ )
    {
        printf( "%s\n", em[i].name );
    }

    return EXIT_SUCCESS;
}

This program has the following behavior:

Number of entries: 5
Enter name: Mike
Enter name: Alice
Enter name: Charlie
Enter name: Bob 
Enter name: Jimmy

Stored the following strings:
Mike
Alice
Charlie
Bob
Jimmy

However, I generally do not recommend using scanf for user input, because it behaves in a non-intuitive manner for line-based user input. As already mentioned, it always leaves the newline character in the buffer, and maybe also some other characters. It is a pain to always have to remove them whenever you call scanf. Also, when using fgets, it is a pain to always have to check for the newline character in order to verify that the entire line was read in, and to remove the newline character.

For this reason, I recommend to write your own functions which handle all of this. In my code below, I call these functions get_int_from_user and get_line_from_user. I have designed these functions in such a way that they also validate the input and reprompt the user, if the input is invalid.

Note that using these functions makes the function main shorter and simpler:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <ctype.h>
#include <limits.h>
#include <errno.h>

int get_int_from_user( const char prompt[] );
void get_line_from_user( const char prompt[], char buffer[], int buffer_size );

#define MAX_ENTRIES 20

typedef struct employee
{
    int code;
    char name[100];
} emp;

int main( void )
{
    emp em[MAX_ENTRIES];
    int n;

    //read number of entries from user
    n = get_int_from_user( "Number of entries: " );

    //verify that number of entries is not so high that it would
    //overflow the buffer
    if ( n > MAX_ENTRIES )
    {
        printf( "Too many entries!\n" );
        exit( EXIT_FAILURE );
    }

    //read individual entries from user
    for ( int i=0; i < n; i++ )
    {
        get_line_from_user(
            "Enter name: ",
            em[i].name, sizeof em[i].name
        );
    }

    //print back the stored strings
    printf( "\nStored the following strings:\n" );
    for ( int i=0; i < n; i++ )
    {
        printf( "%s\n", em[i].name );
    }

    return EXIT_SUCCESS;
}

//This function will attempt to read one integer from the user. If
//the input is invalid, it will automatically reprompt the user,
//until the input is valid.
int get_int_from_user( const char prompt[] )
{
    //loop forever until user enters a valid number
    for (;;)
    {
        char buffer[1024], *p;
        long l;

        //prompt user for input
        fputs( prompt, stdout );

        //get one line of input from input stream
        if ( fgets( buffer, sizeof buffer, stdin ) == NULL )
        {
            fprintf( stderr, "Unrecoverable input error!\n" );
            exit( EXIT_FAILURE );
        }

        //make sure that entire line was read in (i.e. that
        //the buffer was not too small)
        if ( strchr( buffer, '\n' ) == NULL && !feof( stdin ) )
        {
            int c;

            printf( "Line input was too long!\n" );

            //discard remainder of line
            do
            {
                c = getchar();

                if ( c == EOF )
                {
                    fprintf( stderr, "Unrecoverable error reading from input!\n" );
                    exit( EXIT_FAILURE );
                }

            } while ( c != '\n' );

            continue;
        }

        //attempt to convert string to number
        errno = 0;
        l = strtol( buffer, &p, 10 );
        if ( p == buffer )
        {
            printf( "Error converting string to number!\n" );
            continue;
        }

        //make sure that number is representable as an "int"
        if ( errno == ERANGE || l < INT_MIN || l > INT_MAX )
        {
            printf( "Number out of range error!\n" );
            continue;
        }

        //make sure that remainder of line contains only whitespace,
        //so that input such as "6abc" gets rejected
        for ( ; *p != '\0'; p++ )
        {
            if ( !isspace( (unsigned char)*p ) )
            {
                printf( "Unexpected input encountered!\n" );

                //cannot use `continue` here, because that would go to
                //the next iteration of the innermost loop, but we
                //want to go to the next iteration of the outer loop
                goto continue_outer_loop;
            }
        }

        return l;

    continue_outer_loop:
        continue;
    }
}

//This function will read exactly one line of input from the
//user. It will remove the newline character, if it exists. If
//the line is too long to fit in the buffer, then the function
//will automatically reprompt the user for input. On failure,
//the function will never return, but will print an error
//message and call "exit" instead.
void get_line_from_user( const char prompt[], char buffer[], int buffer_size )
{
    for (;;) //infinite loop, equivalent to while(1)
    {
        char *p;

        //prompt user for input
        fputs( prompt, stdout );

        //attempt to read one line of input
        if ( fgets( buffer, buffer_size, stdin ) == NULL )
        {
            printf( "Error reading from input!\n" );
            exit( EXIT_FAILURE );
        }

        //attempt to find newline character
        p = strchr( buffer, '\n' );

        //make sure that entire line was read in (i.e. that
        //the buffer was not too small to store the entire line)
        if ( p == NULL )
        {
            int c;

            //a missing newline character is ok if the next
            //character is a newline character or if we have
            //reached end-of-file (for example if the input is
            //being piped from a file or if the user enters
            //end-of-file in the terminal itself)
            if ( (c=getchar()) != '\n' && !feof(stdin) )
            {
                if ( ferror(stdin) )
                {
                    printf( "Error reading from input!\n" );
                    exit( EXIT_FAILURE );
                }

                printf( "Input was too long to fit in buffer!\n" );

                //discard remainder of line
                do
                {
                    c = getchar();

                    if ( ferror(stdin) )
                    {
                        printf( "Error reading from input!\n" );
                        exit( EXIT_FAILURE );
                    }

                } while ( c != '\n' && c != EOF );

                //reprompt user for input by restarting loop
                continue;
            }
        }
        else
        {
            //remove newline character by overwriting it with
            //null character
            *p = '\0';
        }

        //input was ok, so break out of loop
        break;
    }
}

Here is an example of the behavior of the program:

Number of entries: abc
Error converting string to number!
Number of entries: 6abc
Unexpected input encountered!
Number of entries: 5
Enter name: Mike
Enter name: Alice
Enter name: This is a very long line that is too long to fit into the buffer. This is a very long line that is too long to fit into the buffer.
Input was too long to fit in buffer!
Enter name: Charlie
Enter name: Bob
Enter name: Jimmy

Stored the following strings:
Mike
Alice
Charlie
Bob
Jimmy

As you can see, when invalid input was entered, the input was discarded and the user was reprompted for input.

Andreas Wenzel
  • 22,760
  • 4
  • 24
  • 39