5 ms·

Consider checking the return value of malloc (e.g: https://github.com/doches/progressbar/blob/master/lib/progressbar.c#L22 https://github.com/doches/progressbar

by _RPM 11y ago

Consider checking the return value of malloc (e.g: https://github.com/doches/progressbar/blob/master/lib/progressbar.c#L22 https://github.com/doches/progressbar/blob/master/lib/progre...) You're setting users/yourself up for a disaster if you're assuming it will never fail. The standard [0] states that the size of `char` will be 1 byte. So there is no point in doing the sizeof operator on char. (e.g: https://github.com/doches/progressbar/blob/master/lib/progressbar.c#L28 https://github.com/doches/progressbar/blob/master/lib/progre...) 6.5.3.4 - Paragraph 3 > The sizeof operator... (page 80) [0] http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1124.pdf http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1124.pdf

thinkpad20 11y ago
There's still a point, in terms of readability and consistency. Certainly there doesn't seem to be a downside aside from a few extra characters.
- _RPM 11y ago
  Redundant != consistent. But I agree with you, there isn't really a downside. I'm being pedantic here. -pedantic
  - spash 11y ago
    > I'm being pedantic here. Please be. Seemingly random bits of (honestly - actually very useful) information like yours are why I'm reading HN in the first place instead of, say, random Youtube comments. Someone may argue about this being overly pedantic, or about such comment being more suitable for sites like Stack Overflow, or about the whole thing being completely clear from the standards documentation itself... etc. But that's really not the point of it when this isn't the kind of thing you would be even intentionally looking at, or for, in the first place. Every time I open a HN thread, I can be sure that I will find at least a good few nice and memorable comments that seemingly don't add anything revolutionary to the topic at hand, but instead will make you pause, slap your forehead and exclaim "holy moly, that's in essence actually perfectly obvious, yet somehow I never even thought about it from this point of view before!" Which in turn suddenly makes you realize something completely unrelated in your own world, yet affected or plagued by a similar type of issue, that would otherwise end up completely unnoticed if not for your comment. In short, one man's pedantic can still be other man's sudden epiphany.
  - Doches 11y ago
    This sort of pedantry is exactly why I submitted this library to Show HN in the first place -- I'm not a C programmer by any particular training other than self-taught, and I've never worked at such a low level in industry. Knowing that C programmers consider something like sizeof(char) to be distractingly weird (where I would just consider it consistent/defensive) is good to know!
danieltillett 11y ago
How often is knowing malloc has failed useful? You going to segfault either way. I always use sizeof with char just to stay consistent - if you don’t you will get caught out one day when you change something and forget to add in the needed new sizeof.
- _RPM 11y ago
  It's _very_ important to know if it failed if you don't want to introduce undefined behavior. With undefined behavior, _anything_ can happen, (e.g. arbitrary code execution). But let me make it clear: malloc failing doesn't mean that undefined behavior will happen. However, pretending like malloc didn't fail when it really did will most likely get you undefined behavior. You're not going to segfault either way if you handle things correctly. Consider a web server running out of memory to allocate a request structure, do you think it is a good idea to just let it SIGSEV?
  - danieltillett 11y ago
    If you have run out of memory there is not much you can really do except crash. Even logging is a problem. Better to just crash and deal with it. I was under the impression that compiler optimisation often removed any NULL checks anyway [1]. 1. http://blog.llvm.org/2011/05/what-every-c-programmer-should-know_14.html http://blog.llvm.org/2011/05/what-every-c-programmer-should-...
    - AndyKelley 11y ago
      > I was under the impression that compiler optimisation often removed any NULL checks anyway Goodness. No, you are misunderstanding the example. The optimizer did not introduce a bug by removing a NULL check. The code had a bug because it dereferenced the pointer before checking for NULL.
      - danieltillett 11y ago
        Yes, but they were making the point that thinking you are checking for a null pointer may not always hold especially once inlining happens. Are there any modern platforms where dereferncing a NULL pointer will not cause a crash? Edit. Answering my own question it looks like ARM is one [1]. 1. https://www.securecoding.cert.org/confluence/display/c/EXP34-C.+Do+not+dereference+null+pointers https://www.securecoding.cert.org/confluence/display/c/EXP34...
pjtr 11y ago
Just in case anyone else finds this fascinating: Correct, sizeof(char) is always 1 byte. A byte contains CHAR_BIT bits (page 37). The implementation-defined value of CHAR_BIT is at least 8 (page 21). There are some implementations [1] that chose other values like 16 or 32. So while sizeof(char) is always 1 byte it's not necessarily always 1 octet. [1] http://bytes.com/topic/c/answers/223565-implementations-char_bit-32-a http://bytes.com/topic/c/answers/223565-implementations-char...

by _RPM 11y ago

thinkpad20 11y ago

There's still a point, in terms of readability and consistency. Certainly there doesn't seem to be a downside aside from a few extra characters.

_RPM 11y ago
Redundant != consistent. But I agree with you, there isn't really a downside. I'm being pedantic here. -pedantic
- spash 11y ago
  > I'm being pedantic here. Please be. Seemingly random bits of (honestly - actually very useful) information like yours are why I'm reading HN in the first place instead of, say, random Youtube comments. Someone may argue about this being overly pedantic, or about such comment being more suitable for sites like Stack Overflow, or about the whole thing being completely clear from the standards documentation itself... etc. But that's really not the point of it when this isn't the kind of thing you would be even intentionally looking at, or for, in the first place. Every time I open a HN thread, I can be sure that I will find at least a good few nice and memorable comments that seemingly don't add anything revolutionary to the topic at hand, but instead will make you pause, slap your forehead and exclaim "holy moly, that's in essence actually perfectly obvious, yet somehow I never even thought about it from this point of view before!" Which in turn suddenly makes you realize something completely unrelated in your own world, yet affected or plagued by a similar type of issue, that would otherwise end up completely unnoticed if not for your comment. In short, one man's pedantic can still be other man's sudden epiphany.
- Doches 11y ago
  This sort of pedantry is exactly why I submitted this library to Show HN in the first place -- I'm not a C programmer by any particular training other than self-taught, and I've never worked at such a low level in industry. Knowing that C programmers consider something like sizeof(char) to be distractingly weird (where I would just consider it consistent/defensive) is good to know!

danieltillett 11y ago

How often is knowing malloc has failed useful? You going to segfault either way. I always use sizeof with char just to stay consistent - if you don’t you will get caught out one day when you change something and forget to add in the needed new sizeof.

_RPM 11y ago
It's _very_ important to know if it failed if you don't want to introduce undefined behavior. With undefined behavior, _anything_ can happen, (e.g. arbitrary code execution). But let me make it clear: malloc failing doesn't mean that undefined behavior will happen. However, pretending like malloc didn't fail when it really did will most likely get you undefined behavior. You're not going to segfault either way if you handle things correctly. Consider a web server running out of memory to allocate a request structure, do you think it is a good idea to just let it SIGSEV?
- danieltillett 11y ago
  If you have run out of memory there is not much you can really do except crash. Even logging is a problem. Better to just crash and deal with it. I was under the impression that compiler optimisation often removed any NULL checks anyway [1]. 1. http://blog.llvm.org/2011/05/what-every-c-programmer-should-know_14.html http://blog.llvm.org/2011/05/what-every-c-programmer-should-...
  - AndyKelley 11y ago
    > I was under the impression that compiler optimisation often removed any NULL checks anyway Goodness. No, you are misunderstanding the example. The optimizer did not introduce a bug by removing a NULL check. The code had a bug because it dereferenced the pointer before checking for NULL.
    - danieltillett 11y ago
      Yes, but they were making the point that thinking you are checking for a null pointer may not always hold especially once inlining happens. Are there any modern platforms where dereferncing a NULL pointer will not cause a crash? Edit. Answering my own question it looks like ARM is one [1]. 1. https://www.securecoding.cert.org/confluence/display/c/EXP34-C.+Do+not+dereference+null+pointers https://www.securecoding.cert.org/confluence/display/c/EXP34...

pjtr 11y ago

Just in case anyone else finds this fascinating: Correct, sizeof(char) is always 1 byte. A byte contains CHAR_BIT bits (page 37). The implementation-defined value of CHAR_BIT is at least 8 (page 21). There are some implementations [1] that chose other values like 16 or 32. So while sizeof(char) is always 1 byte it's not necessarily always 1 octet. [1] http://bytes.com/topic/c/answers/223565-implementations-char_bit-32-a http://bytes.com/topic/c/answers/223565-implementations-char...