convert binary to decimal with linked list in C

Question

Hello I have a problem with my code its shows this error: "Segmentation fault: 11" here is my code i'm trying to make a program that convert binary linked list to decimal using lists in C

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

typedef struct cellule{
    int b;
    struct cellule* Next;
}Cellule;

typedef Cellule* liste;

int BinToEntier(liste a){
    int n;
    int j=0;
    liste p=NULL;
    for (p=a; p->Next!=NULL; p=p->Next) {
      n+=(p->b)*pow(2,j);
      j++;
    }
    return n;
}

int main() {
  liste a=NULL;
  liste p;
  a= (liste) malloc(sizeof(Cellule));
  p=a;
  for (int i = 0; i < 4; i++) {
    puts("enter b");
    scanf("%i", &(p->b));
    p=p->Next;
  }
  printf("%i\n", BinToEntier(a));
  return 0;
}

Welcome to Stack Overflow! [Please see this discussion on why not to cast the return value of `malloc()` and family in `C`.](http://stackoverflow.com/q/605845/2173917). — Sourav Ghosh, Dec 22 '16 at 12:26

score 2 · Answer 1 · answered Dec 22 '16 at 12:40

In:

  a= (liste) malloc(sizeof(Cellule));

a is not initialized to zero, yet in your loop you do a p=a;...p=p->Next. This will access undefined memory potentially causing a seg fault. (Note that scanf("%i", &(p->b)); can also cause the seg fault.)

Also in BinToEntier you forget to initialize n.

ilim · Answer 2 · 2016-12-22T15:43:17.020

2

Your for loop in the function main() is causing the segmentation fault. Simply put, you are not allocating space for each node(i.e. Cellule) of the list, but allocating just the first element. Additionally, but almost as equally importantly, your assignment of the last node you append to the tail of the list is erroneous.

Consider switching to this usage given below.

int main() {
    liste p=NULL;
    liste *tp = &p;
    for (int i = 0; i < 4; i++) {
        *tp = malloc(sizeof(Cellule));
        puts("enter b");
        scanf("%i", &((*tp)->b));
        *tp=&((*tp)->Next);
    }
    *tp = NULL;
    printf("%i\n", BinToEntier(p));
    return 0;
}

The code given above uses a variable tp, which is a pointer to a pointer to a Cellule. (i.e. list node) In effect, we update tp to show the "Next" attribute of the last Cellule in the list, and update it as we add new nodes to the list p.

edited Dec 22 '16 at 15:43

answered Dec 22 '16 at 12:43

ilim

4,477
7
27
46

Curious, why does code cast `(liste)` the result of `malloc()`? It certainly is not required, just following OP's example? – chux - Reinstate Monica Dec 22 '16 at 15:39
No particular reasons. It is actually better to not cast it, as can be seen [here](https://stackoverflow.com/questions/605845). I just didn't want to do any other modifications to the code provided by OP. But since you brought it up,let me fix it to use the best practice. – ilim Dec 22 '16 at 15:42
Can certainly understand not wanting to fix too much of OP"s code which has 5+ other weaknesses/problems. – chux - Reinstate Monica Dec 22 '16 at 15:45

convert binary to decimal with linked list in C

2 Answers2