yamt commented on pull request #695: URL: https://github.com/apache/incubator-nuttx-apps/pull/695#issuecomment-835000482
> > in my view: > > assign only when necessary. fix bugs if any. -> strict coding > > make unnecessary assignments to hide potential bugs. -> loose coding > > Sorry, I cannot agree with your view. no problem with having different views. actually it's a good thing of a community. > It is not unnecessary when the motivation for it is to prevent (not hide) a potential bug. And depending on how this opensource code is integrated into a product, this potential bug could escalate into a security vulnerability. the "defensive" style can make it difficult to find such bugs. i know some people like constructs like the following. while i can understand the motivation, my honest impression is that using C was a bad idea for those people in the first place. ``` #define FREE(a) (free(a); a = NULL) #define MALLOC(a) calloc(1, a) ``` > > > but in this particular case, i don't feel it's an improvement. > > It is not an improvement, this is the recommended method for avoiding **use-after-free** and **double-free** errors, both listed in the CWE: > https://cwe.mitre.org/data/definitions/415.html > https://cwe.mitre.org/data/definitions/416.html i certainly don't recommend to apply the method blindly. (i feel it "blindly" because you are suggesting to add "ctx->ws = NULL" even in a place where it's already known to be NULL.) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected]
