Why doesn't this program print the name?

Question

I’m trying to make a program that receives a text from a file where its lines are a first name, a last name and a note. I want to return the ordered lines: first the students with a grade lower than 5 and after the students passed in order of appearance in the file. The problem is that I can’t print the names and I don’t know what the fault is. The main code is as follows:

int main(int argc, char *argv[])
{   
    char line[MaxLinea+1];
    char *name;
    char *surname;
    lista_notas *mi_lista = NULL;
    int i, blank, grade;
    FILE *archivo = fopen(argv[1], "r");

    while(fgets(line, MaxLinea, archivo)) //recorrer linea fich
    {
        grade = get_grade(line);        
        if (grade < 5) //Insertar en la lista
        {
            name = get_name(line);
            surname = get_surname(line);
            insertar(&mi_lista, name, surname, grade);
        }
    }
    
    fclose(archivo);
    archivo = fopen(argv[1], "r");
    
    while(fgets(line, MaxLinea, archivo)) //recorrer linea fich
    {
        grade = get_grade(line);        
        if (grade > 4) //Insertar en la lista
        {
            name = get_name(line);
            surname = get_surname(line);
            insertar(&mi_lista, name, surname, grade);
        }
    }
    
    
    mostrar_lista(mi_lista);
    free_lista(mi_lista);

    //system("leaks a.out");
    return 0;
}

The get_name function is:

char *get_name(char *line)
{
    int i;
    char *name = malloc(51);
    if (name == NULL)
        exit(71);

    i = 0;
    
    while(line[i] != ' ')
        name[i] = line[i++];
        
    name[i] = 0;

    return name;
}

If the program receives a fich.txt which content is:

Nombre1 Apellido1 8
Nombre2 Apellido2 2
Nombre3 Apellido3 6

The output must be:

Nombre2 Apellido2 2
Nombre1 Apellido1 8
Nombre3 Apellido3 6

but instead, it is:

Apellido2 2
Apellido1 8
Apellido3 6

Thanks for your help!

The full code is:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#define MaxLinea 154
//Tamaño máximo linea = 50 (nombre) + 100 (apellido) + 2 (nota máx 10) 
    // + 2 espacios + caracter nulo? = 155

typedef struct notas{
    char *nombre;
    char *apellido;
    int nota;
    struct notas *siguiente;
} lista_notas;

int get_grade(char *line);
char *get_name(char *line);
char *get_surname(char *line);
void insertar(lista_notas **ptr, char *name, char *surname, int grade);
void mostrar_lista(lista_notas *ptr);
void free_lista(lista_notas *ptr);

int main(int argc, char *argv[])
{   
    char line[MaxLinea+1];
    char *name;
    char *surname;
    lista_notas *mi_lista = NULL;
    int i, blank, grade;
    FILE *archivo = fopen(argv[1], "r");

    while(fgets(line, MaxLinea, archivo)) //recorrer linea fich
    {
        grade = get_grade(line);        
        if (grade < 5) //Insertar en la lista
        {
            name = get_name(line);
            surname = get_surname(line);
            insertar(&mi_lista, name, surname, grade);
        }
    }
    
    fclose(archivo);
    archivo = fopen(argv[1], "r");
    
    while(fgets(line, MaxLinea, archivo)) //recorrer linea fich
    {
        grade = get_grade(line);        
        if (grade > 4) //Insertar en la lista
        {
            name = get_name(line);
            surname = get_surname(line);
            insertar(&mi_lista, name, surname, grade);
        }
    }
    
    
    mostrar_lista(mi_lista);
    free_lista(mi_lista);

    //system("leaks a.out");
    return 0;
}


void insertar(lista_notas **ptr, char *name, char *surname, int grade)
{
    lista_notas *p1, *p2;

    p1 = *ptr;
    if(p1 == NULL)
    {
        p1 = malloc(sizeof(lista_notas));//y si usamos calloc ¿qué cambia?
        if (p1 == NULL)
            exit(71);
        if (p1 != NULL)
        {
            p1->nombre = name;
            p1->apellido = surname;
            p1->nota = grade;
            p1->siguiente = NULL;
            *ptr = p1;
        }
    }
    else
    {
        while(p1->siguiente != NULL)//recorrer la lista hasta el último
            p1 = p1->siguiente;
        p2 = malloc(sizeof(lista_notas));//y si usamos calloc ¿qué cambia?
        if (p2 == NULL)
            exit(71);
        if(p2 != NULL)
        {
            p2->nombre = name;
            p2->apellido = surname;
            p2->nota = grade;
            p2->siguiente = NULL;
            p1->siguiente = p2;
        }
    }
}

void mostrar_lista(lista_notas *ptr)
{
    while(ptr != NULL)
    {
        printf("%s ",ptr->nombre);
        printf("%s ",ptr->apellido);
        printf("%d\n",ptr->nota);
        ptr = ptr->siguiente;
    }
/*  printf("\n");*/
}

void free_lista(lista_notas *ptr)
{
    lista_notas *aux = ptr;
    lista_notas *aux2;

    while(aux != NULL)
    {
        aux2 = aux;
        aux = aux->siguiente;
        free(aux2->nombre);
        free(aux2->apellido);
        free(aux2); 
    }
}

int get_grade(char *line)
{
    int i, blank, num;

    i = 0;
    blank = 0;
    while(line[i] != '\0')
    {
        if (blank == 2) //sacar la nota
        {
            num = atoi(&line[i]);
            blank++;
        }
        else if(line[i] == ' ')
            blank++;
        i++;    
    }
    return num;
}

char *get_name(char *line)
{
    int i;
    char *name = malloc(51);
    if (name == NULL)
        exit(71);

    i = 0;
    
    while(line[i] != ' ')
        name[i] = line[i++];
        
    name[i] = 0;

    return name;
}

char *get_surname(char *line)
{
    int i, j;
    char *surname = malloc(101);
    if (surname == NULL)
        exit(71);
    
    i = 0;
    j = 0;

    while(line[i] != ' ')
        i++;

    while(line[++i] != ' ')
        surname[j++] = line[i];
    
    surname[j] = 0;

    return surname;
}

Pls show the functions `get_surname` , `insertar`, and `mostrar_lista`. — collapsar, Jun 30 '22 at 08:04
You have not shown the part of your program that does the output, so we just have to take your word that the function you _think_ is broken is _actually_ the problem. I will say your `get_name` function is dangerous. It assumes the line contains a space (otherwise it will run off the end of the input buffer) and it assumes the word is no longer than 50 characters (otherwise it writes off the end of the output buffer). Why not use `strncpy` or similar for this task? — paddy, Jun 30 '22 at 08:05
First of all try to minimize the program to the shortest code possible that replicate the faulty behavior. Then learn how to [*debug*](https://ericlippert.com/2014/03/05/how-to-debug-small-programs/) your programs. More specifically use a [*debugger*](https://stackoverflow.com/questions/25385173/what-is-a-debugger-and-how-can-it-help-me-diagnose-problems) to step through the code statement by statement while monitoring variables and their values. — Some programmer dude, Jun 30 '22 at 08:07
I'm not 100% on this, so I'll pose it as a question instead: is `name[i] = line[i++];` a potential sequence point issue? — paddy, Jun 30 '22 at 08:09
As `name[i++] = line[i];` would make more sense but this too is undefined behaviour. There is no sequence point between the uses of `i`. — Weather Vane, Jun 30 '22 at 09:09
Regarding the possible UB, your loop in `get_name` can be changed to `for (i = 0; i < 50 && line[i] != '\0' && line[i] != ' '; ++i) { name[i] = line[i]; }` (which also makes sure that you do not go out of bounds of the string or the allocated memory, besides getting rid of the possible UB). — Some programmer dude, Jun 30 '22 at 09:10

chux - Reinstate Monica · Accepted Answer · 2022-06-30T10:23:10.630

At least these problems:

++ not sequenced

name[i] = line[i++]; is bad code. The i++may occur before, after, (or even during) name[i]. Result: undefined behavior (UB). @paddy

Limited size

char *name = malloc(51); does not need to use the magic number 51.

May run off end

while(line[i] != ' ') leads to trouble when line[] lacks a ' '.

Alternative (like OP's code)

char *get_name(char *line) {
    size_t i = 0;
    while(line[i] != ' ' && line[i] != '\0') {
        i++; 
    }

    char *name = malloc(i + 1);
    if (name == NULL) {
        exit(71);
    }

    for (size_t j = 0; j < i; j++) {
        name[j] = line[j];
    }
    name[i] = 0;

    return name;
}

Or perhaps using standard efficient library functions

char *get_name(const char *line) {
  size_t token_length = strcspn(line, " ");
  char *name = malloc(token_length + 1);
  if (name == NULL) {
    exit(71);  // Consider a named macro/enum here rather than a magic 71
  }

  name[token_length] = '\0';
  return memcpy(name, line, token_length);    
}

Other issues

Off by 1

// fgets(line, MaxLinea, archivo)
fgets(line, sizeof line, archivo)

Advanced: Assuming first, last names do not include spaces

Consider first names like "Betty Jo" and last names like Lloyd Webber

Some names are longer than 51.

Why doesn't this program print the name?

1 Answers1