How avoid I a comparation with NULL in strcmp?

Question

I'm tring to make this program work, but after giving it a lot of turns, I decided is time to ask. No matter what I do the result is always a segmentation fault and I pretty much sure that is for the strcmp. Can you help me please?

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

void main ()
{
    char *marcas[7] = {"Alfa Romeo", "Fiat", "Ford", "Lancia", "Renaudl", '\0', '\0'};
    char *marca, *aux;
    int i;
    int cont=5;
    
    marca=" ";
    aux=" ";        
    for(i=0;i<cont;i++)
    printf("%s\n",marcas[i]);
    while (cont<7){
    printf("Ingrese la marca que desee agregar a la lista.\n");
    scanf("%s",marca);
    printf("Gracias. Ud. quiere agregar la marca: %s.\n", marca);
    i=0;
    while(strcmp(marcas[i], marca)<0 && i<cont){
            i++;
            printf("Moviendo puntero.\n");
    }
    printf("Termine de mover.\n");
    if (i < cont){
       while (i<cont){
        strcpy(aux,marcas[i]);
        strcpy(marcas[i],marca);
        strcpy(marca,aux);
        i++;
       }
    } 
    printf("%s tiene que ir en la posicion %d\n", marca, i);
    strcpy(marcas[i],marca);    
    cont++;
    for(i=0;i<cont;i++)
        printf("%s\n", marcas[i]);
    printf("La marca ingresada última a la lista:%s\n", marca);
    }
    for(i=0;i<7;i++)
    printf("%s\n", marcas[i]);
}

Answer 1

Your program contains many segfaults, due to the fact that c does not have built in variable length strings, as many other programming languages do. This can cause problems with trying to 'overfill' a string.

char *marca, *aux;

In this line, you define a pointer (marca and aux) to a single character each, and so when you call this line:

scanf("%s",marca);

you are reading a potentially huge block of text off the command line, and trying to fit it all into the space of one character. This is not good for the program, and if not dealt with can cause vulnerabilities such as Buffer Overflows. C is an inherently unsafe language, and I recommend learning what you are doing before you attempt to use pointers. A better solution might be something like this:

// Create a list of characters (a string) 128 spaces long called marca, and initialize it.
char marca[128] = {'\0'};
char aux[128] = {'\0'};

In conclusion, the best course of action would be to learn C properly.

Answer 2

This statement

scanf("%s",marca);

invokes undefined behavior at least because it is trying to change the string literal pointed to by the pointer marca

marca=" ";

You need to declare a character array where you will read a string.

The same problem exists with the pointer aux used in calls of strcpy

strcpy(aux,marcas[i]);

and with other string literals that are pointed to by elements of the array marcas. SO instead of the array of pointers declare a two-dimensional array initialized by the string literals.

For example

char marcas[7][12] = {"Alfa Romeo", "Fiat", "Ford", "Lancia", "Renaudl", };

Pay attention to that any attempt to change a string literal results in undefined behavior.

Answer 3

One additional note to the other answers is that you can use fgets instead of scanf. The good thing with fgets is that you can limit the number of characters read from the user to whatever you want.

So instead of

scanf("%s", marca);

do

fgets(marca, 128 /*or whatever number you choose*/, stdin);

The above fgets call will only read the first 128 (or whatever number you choose) characters from the user, no matter how many they actually enter. This will prevent buffer overflows.