strcmp causing seg fault in C

Viewed 69

I seem to be getting a seg fault when using strcmp: weirdly the value seems to be the right value stored but still causes a seg fault. Does anyone have any ideas why this is my code?

Say for example I have 'xx' as the value I want to unalias in the 2 lines after the for loop they both = 'xx' but then this line causes a seg fault if (alias_array[i].alias != NULL && strcmp(alias_array[i].alias, arguments[1]) == 0) {

typedef struct alias {
    char alias[50];
    char command[50];
} alias;

alias alias_array[MAX_ALIAS_LEN];

void unalias_command(char *arguments[50]) {
    char alias_name[50];
    strcpy(alias_name, arguments[1]);
    printf("args[1] %s\n", *arguments);
    printf("Len of args = %ld", strlen(arguments[1]));
    //if(strcmp(alias_name, NULL) != 0) {      
    if (arguments != NULL) {
        int found = 0;
        for (int i = 1; i < MAX_ALIAS_LEN; i++) {
            printf("current name = %s\n", alias_array[1].alias);
            printf("current argument = %s\n", arguments[1]);
            if (alias_array[i].alias != NULL && strcmp(alias_array[i].alias, arguments[1]) == 0) {
                // if alias is found and matches then we can use the free() function
                found = 1;
                strcpy(alias_array[i].alias, NULL);
                strcpy(alias_array[i].command, NULL);
                break;
            }
        }
        if (found == 1) {
            printf("Alias, %s has been removed as an alias\n", arguments);
        } else {
            printf("No alias with name %s\n", arguments[1] );
        }
    } else {
        printf("Error: command 'unalias' requires 1 argument"); }
}

... 

char *args[50] = {""}
unalias_command(args)
2 Answers

If you pass an array of strings that contains a single empty string and 50 null pointers:

    char *args[50] = {""}
    unalias_command(args)

You get a segmentation fault here:

   strcpy(alias_name, arguments[1]);

and here:

   printf("Len of args = %ld", strlen(arguments[1]));

Further problems:

  • the members of the alias structure are not pointers, so it makes no sense to compare alias_array[i].alias != NULL
  • strcpy(alias_array[i].alias, NULL); has undefined behavior. You should instead write strcpy(alias_array[i].alias, ""); or simply *alias_array[i] = '\0';

You should pass the count of arguments and the argument array, as main() receives, and you must check for proper usage before proceeding with the command handing:

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

typedef struct alias {
    char alias[50];
    char command[50];
} alias;

alias alias_array[MAX_ALIAS_LEN];

int unalias_command(int argc, char *arguments[50]) {
    char *alias_name;
    if (argc < 2) {
        fprintf(stderr, "unalias: missing argument\n");
        return 1;
    }
    // no need to copy the alias, just use a char pointer
    alias_name = arguments[1];
    if (alias_name[0] != '\0') {
        int found = 0;
        for (int i = 0; i < MAX_ALIAS_LEN; i++) {
            if (strcmp(alias_array[i].alias, alias_name) == 0) {
                // if alias is found and matches then we can reset it
                found = 1;
                *alias_array[i].alias = '\0';
                *alias_array[i].command = '\0';
                break;
            }
        }
        if (found) {
            printf("Alias `%s' has been removed as an alias\n", alias_name);
            return 0;
        } else {
            printf("No alias with name `%s'\n", alias_name);
            return 1;
        }
    }
}

Note that the check if (alias_array[i].alias != NULL) is actually not doing anything meaningful. The .alias member is declared as a fixed char[50] array so, in that expression, it will decay to a pointer (to the first element of that array). That cannot be NULL (in any meaningful dataset).

You need some other method for testing if the alias_array[i].alias member represents a valid string. Possibly, if (alias_array[i].alias[0] != '\0').

Related