5 ms·

The code snippet is incorrect. I do not see a check on malloc returning null. In general that would be okay, but in an embedded environment with limited memory,

The code snippet is incorrect. I do not see a check on malloc returning null. In general that would be okay, but in an embedded environment with limited memory, that would be a serious problem. Also, using recursion is bad, because copying a long list will lead to a stack overflow.

omnicognate 5y ago
It's never OK. Machines with lots of memory run out of memory too. To present a piece of research in such trollish terms, accusing "stubborn C developers" of causing security vulnerabilities, and then present an example of code for which "we didn't even have to bother reasoning about how exactly it did its pointer manipulations" that segfaults if it runs out of memory (it dereferences the result of malloc and writes to that location 2 lines down) is beyond hilarious.
- bestouff 5y ago
  On Linux machines malloc()'s returning NULL doesn't mean you're out of memory, it means you're out of address space which is very different.
  - fjfaase 5y ago
    If I understand it correctly, it is even worse, because malloc could return not NULL and still throw an exception when you try to write to the 'allocated' memory. See for example https://linuxembedded.fr/2020/01/overcommit-memory-in-linux https://linuxembedded.fr/2020/01/overcommit-memory-in-linux
  - omnicognate 5y ago
    In a system with virtual memory there are a variety of limits you can hit that will prevent your process allocating more memory and cause malloc to return zero. It's normal to informally lump all these together as "running out of memory" when the distinction doesn't matter, as here. Address space size is one of these limits, not the only one. Hitting it is rare on 64-bit operating systems, common on 32-bit ones.
    - kaba0 5y ago
      Unfortunately, I’ve yet to come across a major application on linux that handles this condition gracefully.
- bsenftner 5y ago
  The tone and example code is ludicrous. If the expressed attitude is how they perceive what they are doing, their attitudes are going to be the reason their project fails.
jbluepolarbear 5y ago
The first line doesn’t check if is the void **r pointer is null before the dereference. The incorrectness starts there.
- Jweb_Guru 5y ago
  If the precondition states that the pointer is valid (which I expect it does), then this is not incorrect. If it does not state anything about the pointer, then just checking for NULL isn't sufficient. It'd have to explicitly state "the pointer is either valid for the duration of the function, or NULL" for this to be a worthwhile check to perform.
  - deleted 5y ago
    [deleted]
  - gopiandcode 5y ago
    Yes, that is correct. The precondition for the function states that the pointer should not be null. The specification for the listcopy is listed slightly lower down on the article as follows: // r :-> x ** linked_list(x,S) void listcopy(void **r) // r :-> y ** linked_list(y,S) ** linked_list(x,S) As the precondition explicitly states that r points to a value x, it can not be NULL, so no NULL checks are needed. I'm not sure what the sibling post is trying to imply with recursive calls though.
    - vidarh 5y ago
      > I'm not sure what the sibling post is trying to imply with recursive calls though. I think that is based on a misunderstanding of what it being passed in at the point listcopy() is called in the body of listcopy(). The pointer passed in there can't be NULL due to how it is obtained, so no further checks is necessary.
    - omnicognate 5y ago
      AFAICT the recursion doesn't mean the function can segfault due to a null pointer. It does however mean that it will cause a stack overflow if the list is sufficiently large, which makes it a bad implementation. It can also segfault if malloc returns zero as discussed above. Apart from these serious runtime behaviour flaws the code is also violently unreadable, which you strangely use as a way of attacking C programmers generally, as if this code is representative of human-coded C. This implementation would fail code review for numerous reasons anywhere I've ever worked. The article itself looks detailed and very interesting and I'll have a proper read of it if I get the time. You didn't need to make it so aggressive and clickbaity.

omnicognate 5y ago

It's never OK. Machines with lots of memory run out of memory too. To present a piece of research in such trollish terms, accusing "stubborn C developers" of causing security vulnerabilities, and then present an example of code for which "we didn't even have to bother reasoning about how exactly it did its pointer manipulations" that segfaults if it runs out of memory (it dereferences the result of malloc and writes to that location 2 lines down) is beyond hilarious.

bestouff 5y ago
On Linux machines malloc()'s returning NULL doesn't mean you're out of memory, it means you're out of address space which is very different.
- fjfaase 5y ago
  If I understand it correctly, it is even worse, because malloc could return not NULL and still throw an exception when you try to write to the 'allocated' memory. See for example https://linuxembedded.fr/2020/01/overcommit-memory-in-linux https://linuxembedded.fr/2020/01/overcommit-memory-in-linux
- omnicognate 5y ago
  In a system with virtual memory there are a variety of limits you can hit that will prevent your process allocating more memory and cause malloc to return zero. It's normal to informally lump all these together as "running out of memory" when the distinction doesn't matter, as here. Address space size is one of these limits, not the only one. Hitting it is rare on 64-bit operating systems, common on 32-bit ones.
  - kaba0 5y ago
    Unfortunately, I’ve yet to come across a major application on linux that handles this condition gracefully.
bsenftner 5y ago
The tone and example code is ludicrous. If the expressed attitude is how they perceive what they are doing, their attitudes are going to be the reason their project fails.

jbluepolarbear 5y ago

The first line doesn’t check if is the void **r pointer is null before the dereference. The incorrectness starts there.

Jweb_Guru 5y ago
If the precondition states that the pointer is valid (which I expect it does), then this is not incorrect. If it does not state anything about the pointer, then just checking for NULL isn't sufficient. It'd have to explicitly state "the pointer is either valid for the duration of the function, or NULL" for this to be a worthwhile check to perform.
- deleted 5y ago
  [deleted]
- gopiandcode 5y ago
  Yes, that is correct. The precondition for the function states that the pointer should not be null. The specification for the listcopy is listed slightly lower down on the article as follows: // r :-> x ** linked_list(x,S) void listcopy(void **r) // r :-> y ** linked_list(y,S) ** linked_list(x,S) As the precondition explicitly states that r points to a value x, it can not be NULL, so no NULL checks are needed. I'm not sure what the sibling post is trying to imply with recursive calls though.
  - vidarh 5y ago
    > I'm not sure what the sibling post is trying to imply with recursive calls though. I think that is based on a misunderstanding of what it being passed in at the point listcopy() is called in the body of listcopy(). The pointer passed in there can't be NULL due to how it is obtained, so no further checks is necessary.
  - omnicognate 5y ago
    AFAICT the recursion doesn't mean the function can segfault due to a null pointer. It does however mean that it will cause a stack overflow if the list is sufficiently large, which makes it a bad implementation. It can also segfault if malloc returns zero as discussed above. Apart from these serious runtime behaviour flaws the code is also violently unreadable, which you strangely use as a way of attacking C programmers generally, as if this code is representative of human-coded C. This implementation would fail code review for numerous reasons anywhere I've ever worked. The article itself looks detailed and very interesting and I'll have a proper read of it if I get the time. You didn't need to make it so aggressive and clickbaity.