Re: Makefile vs Make.defs
I mention avoiding targets because most developers don't notice, as I didn't, that the upper level makefile is included at the end so any targets should follow that inclusion. Given how little many devs no about Makefiles that little tidbit becomes difficult to debug. Specifically any target added before the inclusion becomes the default target, unless the target starts with a '.'. On 12/31/2023 10:13 PM, yfliu2008 wrote: Bill, Sorry that I just read your emails and tried what you told. By using @echo directly, I saw "rule fired" printed out, just my other comamnd doesn't run yet, here is the part of my board level Makefile where I appended the double-colon rule: ... include $(TOPDIR)/boards/Board.mk .PHONY: clean clean:: @echo "rule fired" >&2 $(call DELFILE romfs_boot.c) It seems that the $(call DELFILE ...) command doesn't run, but later I noticed that I missed a "," after DELFILE so after fixing that, it seems working now at my side. Here my "clean::" target lives after the Board.mk inclusion, is this right? Do you still suggest not using any targets in the board level Makefile? BTW, I am using config "canmv230/knsh" here. The upstream Makefile doesn't have the double-colon target yet, thus it won't clean the generated "romfs_boot.c" even if one run clean., but this isn't a big issue as I found most time rebuilt kernel can load user land apps in the ROMFS defined by an existing "romfs_boot.c". Regards, yf Original From:"Bill Rees"< redskyo...@icloud.com.INVALID >; Date:2024/1/1 10:49 To:"dev"< dev@nuttx.apache.org >; Subject:Re: Makefile vs Make.defs Well I screwed up on this. The makefile actually runs before including Board.mk which contains the real targets so the default target becomes the one in the src/Makefile. Fundamentally this Makefile should not contain any targets. On 12/31/2023 6:46 PM, Bill Rees wrote: > > Out of curiosity I followed your example and added a clean:: to a > src/Makefile. The target fired. > > :: git diff > diff --git a/boards/arm/efm32/efm32-g8xx-stk/src/Makefile > b/boards/arm/efm32/efm32-g8xx-stk/src/Makefile > index dc8938..79998ed009 100644 > --- a/boards/arm/efm32/efm32-g8xx-stk/src/Makefile > +++ b/boards/arm/efm32/efm32-g8xx-stk/src/Makefile > @@ -28,4 +28,7 @@ else > CSRCS += efm32_userleds.c > endif > > +clean:: > + @echo "=" >&2 > + > include $(TOPDIR)/boards/Board.mk > > How does your setup differ from this? I had already built nuttx once. > > On 12/31/2023 12:33 PM, Bill Rees wrote: >> >> The command to your rule, clean:: does not need a shell process >> wrapper. >> >> You don't need the $() to execute shell commands in the recipe >> since make does that for you. >> >> Change the echo to simply: echo "board level clean" >&2 >> >> The '>&2' directs the output of echo to the stderr file >> descriptor for your process and should outwit most attempts to >> capture output any recipe uses before hand. Running make can produce >> so much output that many times its run silent which means your output >> maybe suppressed. >> >> What board are you working on? >> >> On 12/30/2023 8:03 PM, yfliu2008 wrote: >>> Bill, >>> >>> >>> Thank you. >>> >>> >>> It seems that there is a "clean::" in "boards/Board.mk" which is >>> included by my board level Makefile. >>> The double-colon target thing sounds nice, so I added one to my >>> board level Makefile like below: >>> >>> >>> clean:: >>> $(echo "board level clean") >>> >>> >>> Then when I tried "make clean" under $(TOPDIR), there is no more >>> errors but the rule I added didn't fire. >>> >>> >>> >>> >>> >>> >>> I guess something else is still missing. The top level makefile >>> doesn't trigger my board level double-colon target. >>> >>> >>> >>> Regards, >>> yf >>> >>> >>> >>> >>> Original >>> >>> From:"Bill Rees"< redskyo...@icloud.com.INVALID >; >>> >>> Date:2023/12/31 9:51 >>> >>> To:"dev"< dev@nuttx.apache.org >; >>> >>> Subject:Re: Makefile vs Make.defs >>> >>> >>> >>> You're welcome. >>> >>> The error message is pointing out that you have two targets for clean. >>> One is clean: and the other is clean:: >>> >>> You may be getting a clean target from an included makefile such as a >>> Make.def which may be in a directory up the path. Try issuing a grep >>> for >>> clean: to see what files contain it. Something like "grep -r clean.*:" >>> You may have to move a dir or two up the path. >>> >>> You may wish to try changing the syntax of your target to use the colon >>> syntax of the other. E.g. :: -- : or vice versa. Make allows multiple >>> instances of a target in some circumstances. >>> >>> Another way to get around this is to add the target as another name and >>> perform what you need to do. E.g. instead of clean use myclean: >>> >>> Please get back with your results. >>> >>> Bill >>> >>> On 12/
Re: debugassert vs assert in apps
Here is the issue to track debug.h created before: https://github.com/apache/nuttx/issues/8986 On Wed, Jan 3, 2024 at 2:30 AM Gregory Nutt wrote: > > On Tue, Jan 2, 2024 at 11:16 AM Fotis Panagiotopoulos > > wrote: > >> 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). > > We have no way to enforce this. What we do do is at least make it clear > that the application is using a kernel-only macro or function > inappropriately by putting the macro definition or function prototype in > NuttX private file. For example, > > * Remove the DEBUGASSERT (and ASSERT?) macros from include/assert.h > * Add them to include/nuttx/assert.h > * All applications that use DEBUGASSERT (or ASSERT) should be changed > to use assert() > * Kernel functions that use DEBUGASSERT (or ASSERT) would have to > explicitly include > > Not foolproof, but that is how we have dealt this this in the past. It > if is in include/assert.h, is is essentially being advertised as OK for > use by applications. This is logically equivalent to the Linux/LibC > conventions of putting Linux-specific files in /usr/include/linux vs. > normal, standard application files in /usr/include. > > There may be some issues in common application/kernel LibC files that > use DEBUGASSERT (or ASSERT) > > This same discussion should also apply to include/debug.h which contains > only nonstandard debug macros. Shouldn't it also be moved to > include/nuttx/debug.h? > >
Re: debugassert vs assert in apps
On Tue, Jan 2, 2024 at 11:16 AM Fotis Panagiotopoulos wrote: 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). We have no way to enforce this. What we do do is at least make it clear that the application is using a kernel-only macro or function inappropriately by putting the macro definition or function prototype in NuttX private file. For example, * Remove the DEBUGASSERT (and ASSERT?) macros from include/assert.h * Add them to include/nuttx/assert.h * All applications that use DEBUGASSERT (or ASSERT) should be changed to use assert() * Kernel functions that use DEBUGASSERT (or ASSERT) would have to explicitly include Not foolproof, but that is how we have dealt this this in the past. It if is in include/assert.h, is is essentially being advertised as OK for use by applications. This is logically equivalent to the Linux/LibC conventions of putting Linux-specific files in /usr/include/linux vs. normal, standard application files in /usr/include. There may be some issues in common application/kernel LibC files that use DEBUGASSERT (or ASSERT) This same discussion should also apply to include/debug.h which contains only nonstandard debug macros. Shouldn't it also be moved to include/nuttx/debug.h?
Re: debugassert vs assert in apps
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 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 > 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 > > *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 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 > > > 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 o
Re: debugassert vs assert in apps
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 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 > *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 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 > > 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
