Strcat throws segmentation fault on simple getch-like password input

Question

I am using Linux and there is a custom function of which returns an ASCII int of current key sort of like getch(). When trying to get used to it and how to store the password I came into an issue, my code is as follows:

int main() {
    int c;
    char pass[20] = "";

    printf("Enter password: ");
    while(c != (int)'\n') {
        c = mygetch();
        strcat(pass, (char)c);
        printf("*");
    }

    printf("\nPass: %s\n", pass);

    return 0;
}

Unfortunately I get the warning from GCC:

pass.c:26: warning: passing argument 2 of ‘strcat’ makes pointer from integer without a cast
/usr/include/string.h:136: note: expected ‘const char * __restrict__’ but argument is of type ‘char’

I tried using pointers instead of a char array for pass, but the second I type a letter it segfaults. The function works on its own but not in the loop, atleast not like getch() would on a Windows system.

What can you see is wrong with my example? I am enjoying learning this.

EDIT: Thanks to the answers I came up with the following silly code:

int c;
int i = 0;
char pass[PASS_SIZE] = "";

printf("Enter password: ");
while(c != LINEFEED && strlen(pass) != (PASS_SIZE - 1)) {
    c = mygetch();
    if(c == BACKSPACE) {
        //ensure cannot backspace past prompt
        if(i != 0) {
            //simulate backspace by replacing with space
            printf("\b \b");
            //get rid of last character
            pass[i-1] = 0; i--;
        }
    } else {
        //passed a character
        pass[i] = (char)c; i++;
        printf("*");
    }
}
pass[i] = '\0';
printf("\nPass: %s\n", pass);

Answer 1

The problem is that strcat expects a char * as its second argument (it concatenates two strings). You don't have two strings, you have one string and one char.

If you want to add c to the end of pass, just keep an int i that stores the current size of pass and then do something like

pass[i] = (char) c.

Make sure to null-terminate pass when you are done (by setting the last position to 0).

Answer 2

A single character is not the same as a string containing a single character.

In other words, 'a' and "a" are very different things.

A string, in C, is a null-terminated array of chars. Your "pass" is an array of 20 chars - a block of memory containing space for 20 chars.

The function mygetch() returns a char.

What you need to do is to insert c into one of the spaces.

Instead of "strcat(pass, c)", you want to do "pass[i] = c", where i starts at zero, and increments by one for every time you call mygetch().

Then you need to do a pass[i] = '\0', when the loop is done, with i equal to the number of times you called mygetch(), to add the null terminator.

You're other problem is that you haven't set a value for c, the first time you check to see if it's '\n'. You want to call mygetch() before you do the comparison:

int i = 0;
for (;;)
{
    c = mygetch();
    if (c == '\n')
        break;

    c = mygetch();
    pass[i++] = c;
}
pass[i] = '\0';

Answer 3

Over and above the correctly diagnosed issue with strcat() taking two strings -- why did you ignore the compiler warnings, or if there were no warnings, why don't you have warnings turned on? As I was saying, over and above that problem, you also need to consider what happens if you get EOF, and you also need to worry about the initial value of 'c' (which could accidentally be '\n' though it probably isn't).

That leads to code like this:

int  c;
char pass[20] = "";
char *end = pass + sizeof(pass) - 1;
char *dst = pass;

while ((c = getchar()) != EOF && c != '\n' && dst < end)
    *dst++ = c;
*dst = '\0';  // Ensure null termination

I switched from 'mygetch()' to 'getchar()' - primarily because what I say applies to that and might not apply to your 'mygetch()' function; we don't have a specification of what that function does on EOF.

Alternatively, if you must use strcat(), you still need to keep a track on the length of the string, but you can do:

char c[2] = "";
char pass[20] = "";
char *end = pass + sizeof(pass) - 1;
char *dst = pass;

while (c[0] !=  '\n' && dst < end)
{
    c[0] = mygetch();
    strcat(dst, c);
    dst++;
}

Not as elegant as all that - using strcat() in context is overkill. You could, I suppose, do simple counting and repeatedly use strcat(pass, c), but that has quadratic behaviour as strcat() has to skip of 0, 1, 2, 3, ... characters on the subsequent iterations. By contrast, the solution where dst points to the NUL at the end of the string means that strcat() doesn't have to skip anything. With a fixed size addition of 1 character, though, you're probably better off with the first loop.