0

I have the following code:

if ((ptCurEntry->pNext = (TISOMStscBoxEntry *) malloc(sizeof(TISOMStscBoxEntry))) == NULL)
{
    return ERR_OUT_OF_MEMORY;
}
ptCurEntry->pNext->pNext = NULL;

I malloc a space which is the size of TISOMStscBoxEntry. In this structure, there is a pointer pNext included. In normal case, ptCurEntry->pNext->pNext = NULL is worked. (Just assign NULL to that created pointer) However, I got segmentation fault which was caused by ptCurEntry->pNext->pNext = NULL when the system was busy. It seems like the error handling above for malloc is fine, what's wrong with it? Maybe I cannot rely on the returned NULL of malloc?

Ryan Yang
  • 71
  • 8
  • 4
    Note: Please [see why not to cast](http://stackoverflow.com/q/605845/2173917) the return value of `malloc()` and family in `C`. – Sourav Ghosh Aug 19 '15 at 09:06
  • @nos maybe that `ptCurEntry->pNext = ` is a clue to your first question. – WhozCraig Aug 19 '15 at 09:11
  • What you mean with "when system is busy"? Happens **only** when system is busy? – Frankie_C Aug 19 '15 at 09:15
  • What's the value of `ptCurEntry` just before you execute this code? – barak manos Aug 19 '15 at 09:22
  • Most likely ptCurEntry isn't valid. There's not necessarily any relation between the line that contains the bug and the line where the program crashes. At any rate, the bug is located outside the code posted. – Lundin Aug 19 '15 at 09:23
  • 1
    Also, unless you get paid per number of parenthesis written, consider changing the code to `ptCurEntry->pNext = malloc(sizeof(TISOMStscBoxEntry)); if(ptCurEntry->pNext == NULL)` – Lundin Aug 19 '15 at 09:24
  • A comment about the first comment (why not to cast the return value of `malloc`)? Does this really help answering the question? Or do people on SO just like to show that they know this general fact, which although correct, doesn't seem to contribute whatsoever in explaining `malloc` bugs (even according to the accepted answer in that link, the only possible runtime issue is if `stdlib.h` is not included). It seems very often that users here like boasting with quotes such as "undefined behavior" and "don't cast the return-value of `malloc`", instead of concentrating in the question at hand!!!!! – barak manos Aug 19 '15 at 09:43

2 Answers2

2

Things to check

  • ptCurEntry points at a valid object. If this is not so, accessing ptCurEntry->pNext will give undefined behaviour;
  • ptCurEntry->pNext is actually a pointer to TISOMStscBoxEntry. If this is not true, it is possible not enough memory is being allocated.
  • Before the call of malloc(), <stdlib.h> has been included. Failure to do this can result dereferencing the pointer to have undefined behaviour.

Also, change the

if ((ptCurEntry->pNext = (TISOMStscBoxEntry *) malloc(sizeof(TISOMStscBoxEntry))) == NULL)

to

if ((ptCurEntry->pNext = malloc(sizeof(*(ptCurEntry->pNext)))) == NULL)

This addresses some of the things to check above. If it does not compile it means that either you have omitted to #include <stdlib.h> or that your compiler is a C++ compiler rather than a C compiler.

I am assuming your code is not multithreaded.

Peter
  • 35,646
  • 4
  • 32
  • 74
0

malloc can behave abnormally or generate core if your application has already made any memory corruption. So make sure there is no invalid read/write with valgrind or similar tools

Jossy Sebastian
  • 149
  • 4