This is an excellent summary! I suggest this should be documented somewhere in Documentation, with a more summarized version in the comment block of these macros. This way, the discussion and its conclusions will not be forgotten in the future!!
Happy New Year, Nathan On Tue, Jan 2, 2024 at 11:16 AM Fotis Panagiotopoulos <f.j.pa...@gmail.com> wrote: > > Hello everyone, and happy new year! > > After discussing this with Tim, I would like to present my opinion on this > topic, in the hope we build better code conventions > (or maybe understand better how apps are developed in Nuttx, as I don't use > them often). > > So the idea is: > > DEBUGASSERT shall only be used for the kernel. > assert shall only be used for apps. > > Rationale: > > DEBUGASSERT is a custom macro that only exists in the NuttX world. > As such it is best being used in NuttX-specific code. > > assert on the other hand is a standard function, it exists in all systems. > It seems better suited for generic and portable code (i.e. apps). > > Benefits of this approach: > > 1. Kernel and apps development can be isolated. > It is very common to work only on one of the two parts of the system at a > time. > In some cases, different people (or teams) work on these two different > codebases, > so it is advantageous to only enable what you actually care for. > > 2. assert is more portable. > Generic applications shall remain portable (in the POSIX domain), and not > have dependencies on > "custom things". So one can easily grab a NuttX app and run it on another > system (e.g. Linux). > Or get an external library, not meant for NuttX, and run it directly and > unaltered. > I see minimal value in adding OS-specific macros in generic code. > > 3. The two macros may have different implementations. > Having two different macros with different scopes may allow as in the > future to have them implemented > optimally for their use case. > For example, DEBUGASSERT may provide more OS-specific information about the > time of crash, > reboot the system etc. > On the other hand assert may only terminate the offending task, or provide > more task-specific information etc. > Of course, I know how complicated this is. For the time I only discuss > about keeping the distinction and having > the future ability. > > 4. Code readability / maintainability. > Mixing up assert and DEBUGASSERT in the same code (i.e. in an app), is very > confusing. > I don't understand, why use one or the other? Are they different? Why be > inconsistent? > If they are identical now, will they remain so forever? > What if someone needs to make OS-specific changes to DEBUGASSERT. > > 5. Static code analysis. > There are several good static code analysis tools, and I use some of them. > They can mostly understand assert but not the custom DEBUGASSERT macro. > This causes headaches to re-define correctly DEBUGASSERT in the scope of > the static analysis tool. > Using non-standard macros I have seen checks to fail, or cause false > positives / negatives: > * About unused variables (that are only used in the asserts). > * About asserts with side effects (the tool does not check properly custom > macros) > * About NULL pointer dereferences (that however are checked by asserts). > and more... > > > > On Sun, Dec 17, 2023 at 7:04 AM Daniel Appiagyei > <daniel.appiag...@braincorp.com.invalid> wrote: > > > While we’re on this topic I would also like to encourage authors to put > > only one condition in their asserts instead of multiple. > > So, DEBUGASSERT(cond1) > > ; DEBUGASSERT (cond2); (…) > > > > Instead of DEBUGASSERT(cond1 && cond2 &&..). > > > > When an assertion happens and there’s multiple conditions then I don’t know > > which one triggered it. > > Best, > > Daniel > > > > > > *Daniel Appiagyei | Embedded Software Engineer *Email: > > daniel.appiag...@braincorp.com > > <bog...@braincorporation.com>*Brain* > > * Corp™ *10182 Telesis Ct, Suite 100 > > San Diego, CA 92121 > > > > (858)-689-7600 > > www.braincorp.com > > > > > > On Fri, Dec 15, 2023 at 3:40 AM Alan C. Assis <acas...@gmail.com> wrote: > > > > > I think debugassert() is for finding BUGs during the development/testing > > > phase. > > > > > > In the other hand assert() should be put in case where something really > > > catastrofic is going to happen and cannot be avoid anyway. > > > > > > My personal opinion is that release code shouldn't have (or should have > > the > > > minimum as possible) assert() and PANIC(). You don't wasn't want your > > code > > > to fail in a critical moment, do you? > > > > > > So assert() code could be used during an inicial validation phase in the > > > field. But of course, if it is a HW issue, there is nothing your code can > > > do except showing a BSOD. > > > > > > BR, > > > > > > Alan > > > > > > On Thu, Dec 14, 2023 at 11:50 AM Tim Hardisty <timhardist...@gmail.com> > > > wrote: > > > > > > > Hi, > > > > > > > > I'm wondering if there is an "approved" usage of assert vs debugassert > > > > for files/apps in the nuttx-apps repo? > > > > > > > > My thinking is: > > > > > > > > * use debugassert if a function in apps would only fail if the > > calling > > > > code, probably (or possibly only) if other functions within apps, > > > > rather than an unknown higher level app, have done something dumb > > > > that should be picked up during nuttx-apps development and so > > should > > > > not make it through to final code; but if they do, that's the fault > > > > of the developer for not turning on debug asserts!! > > > > * use assert if the error would cause the obviously unknown > > > > higher-level calling app someone's writing to fail if the error was > > > > allowed to pass unchecked, so the developer of that higher level > > app > > > > can deal with it programmatically. > > > > > > > > That might almost suggest that debugassert() is for private functions > > > > and assert() is for public ones? > > > > > > > > Probably over-thinking it as usual, but I'm never afraid to ask :) > > > > > > > > Thanks! > > > > > > > > TimJTi. > > > > > > > > >
