0

I may have an error in my code but just by commenting an int asignment that is not even executed is the difference between a segmentation fault and a successfull run of the program. Why does this happen?

C code:

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

//keep it > 0
#define DEFAULT_SIZE 50

struct List* init_list();

struct InternalString{
  struct List* l;
};
struct List{
  struct List * prev;
  int max_pos;
  int curr_pos;
  char value[];
};
typedef struct{
  int len;
  struct InternalString* __internal__;
} String;
// string constructor
String build(){
  String str;
  // creates List with an array of max length = DEFAULT_SIZE
  struct List* l = init_list(DEFAULT_SIZE);
  struct InternalString inter;
  inter.l = l;
  str.__internal__ = &inter;
  str.len = 0;
  return str;
}

struct List* init_list(int length){
  struct List* l = malloc((length * sizeof(char))+sizeof(struct List));
  l->curr_pos = -1;
  l->max_pos = length-1;
  l->prev = NULL;
  return l;
}

void push(char* str, String src){
  struct List* l = src.__internal__->l;
  int space = l->max_pos - l->curr_pos;
  int i = 0;

  for (; i < l->max_pos; i++){
    if (str[i] == '\0'){
      printf("RETURNED\n"); // <---------- Run with the problematic line commented to see that it returns here.
      return;
    }
    l->value[++(l->curr_pos)] = str[i];
  }
  int needed_space;
  //int i_clone = i; // <---------- uncommenting this line provoques the segmentation fault
}
// tests
int main(){
  String st = build();
  push("defg", st);
}

I'm compiling with gcc 9.4.0 on ubuntu

user438383
  • 5,716
  • 8
  • 28
  • 43
Ubuntu4Life
  • 11
  • 2
  • 3
    It doesn't. The assignment isn't the cause of your problem, it is just altering how or if symptoms appear. Don't get hung up on it. – Avi Berger Aug 21 '22 at 05:08
  • 3
    Local variables exist for only until they go out of scope. In `build()` you have a local variable `struct InternalString inter;` You set a member variable of your return value (str) to point to it. So `main()` gets a pointer that is set to an invalid value & is illegal to use. That is a definite bug, but not necessarily the only one. I haven't looked through everything. I'd suggest you review the topic of 'object lifetime' – Avi Berger Aug 21 '22 at 05:17
  • 1
    I suggest that you always print diagnostic messages such as `printf("RETURNED\n");` to `stderr` instead of `stdout`, because `stdout` is buffered and therefore won't necessarily be printed when a segmentation fault occurs shortly afterwards. However, if you are sure that `stdout` is line bufffered in your case, then printing the newline character should be sufficient to ensure that the buffer is flushed. – Andreas Wenzel Aug 21 '22 at 05:20
  • 1
    As you're discovering, C++ is very powerful, but also very unforgiving of mistakes. I'd strongly recommend that you use tools like cppcheck and valgrind to validate even the simplest piece of code you write, and help you catch errors such as this. – Buffoonism Aug 21 '22 at 05:40
  • 1
    Off topic for the SEGFAULT: You `malloc()` space then set `maxpos` to be 1 less than length. Are you expecting to deal with the buffer as a "null terminated string" eventually? There's no '\0' being written to the buffer that holds the copies of presumably appended "strings"... – Fe2O3 Aug 21 '22 at 05:43
  • 2
    If you are using the compilers gcc or clang, I recommend that you compile with `-fsanitize=address,undefined`. This will make your program significantly slower, but will often detect when your program is doing something it is not supposed to do, and will provide an immediate error message. – Andreas Wenzel Aug 21 '22 at 05:44
  • 1
    `__internal__` is a [reserved identifier](http://port70.net/~nsz/c/c11/n1570.html#7.1.3p1): "All identifiers that begin with an underscore and either an uppercase letter or another underscore are always reserved for any use. ... If the program declares or defines an identifier in a context in which it is reserved (other than as allowed by 7.1.4), or defines a reserved identifier as a macro name, the **behavior is undefined**." – Andrew Henle Aug 21 '22 at 11:31
  • `struct InternalString* __internal__;` Change to `struct InternalString internal`. No pointer, you don't need it. Also no double underscores --- you cannot use such names, it is undefined behaviour. Change the other places accordingly. – n. m. could be an AI Aug 21 '22 at 11:31

1 Answers1

0

This means that there is a Memory Access Violation. Sometimes the program can still work, or in other cases not. Even a simple change, like adding a line, can make the difference - between working and Segmentation Fault. Anyway, with or without the line, your code causes such a Memory Access Violation.
You could

  • write simpler code
  • search the error by simple methods, like commenting various parts and printf
  • use debugging tools Other options:
  • Just for checking: increase some sizes of allocated memory one after another, and check if there is a difference (might not always help to find the problem)
  • Allocate more memory, e.g. +2 sizes, one before and one after the end of the used array. Plus write special (unusual) values to the surrounding array elements. Whenever such a surrounding array element is overwritten, that indicates the error then.

In your case, your code does not invite to look at it, at all. Why? It appears to be mixed up to a large extent. Please structure your coding document. Really. (e.g. one part with structures, one with subroutines, not all mixed together)

In build() the variable inter is lost, after leaving build()!

I used

gcc -fsanitize=address,undefined main.c

like provided by very helpful comment.

The indicated memory leak disappears, when defining inter as static by

static struct InternalString inter;

(this might not be what you want, but this indicates were one memory leak is).

I have a question to you, what is this?

struct List* l = malloc((length * sizeof(char))+sizeof(struct List));

If you have a structure, then you allocate the associated structure just by any_number * sizeof(structure). Why do you mix a sizeof(char) into that? Am i missing something - can somebody help? ---> OK, i found this Why is undefined size array in struct allowed? ---> Personal note: Well, I would propose, rather use a fixed array length or, if no upper limit of length is clear, use a pointer.

sidcoder
  • 460
  • 2
  • 6