<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Feb 18, 2021 at 2:08 PM Gedare Bloom <<a href="mailto:gedare@rtems.org">gedare@rtems.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Thu, Feb 18, 2021 at 12:52 PM Joel Sherrill <<a href="mailto:joel@rtems.org" target="_blank">joel@rtems.org</a>> wrote:<br>
><br>
><br>
><br>
> On Thu, Feb 18, 2021 at 11:52 AM Gedare Bloom <<a href="mailto:gedare@rtems.org" target="_blank">gedare@rtems.org</a>> wrote:<br>
>><br>
>> On Thu, Feb 18, 2021 at 10:20 AM Joel Sherrill <<a href="mailto:joel@rtems.org" target="_blank">joel@rtems.org</a>> wrote:<br>
>> ><br>
>> ><br>
>> ><br>
>> > On Thu, Feb 18, 2021 at 11:06 AM Gedare Bloom <<a href="mailto:gedare@rtems.org" target="_blank">gedare@rtems.org</a>> wrote:<br>
>> >><br>
>> >> On Thu, Feb 18, 2021 at 8:56 AM Joel Sherrill <<a href="mailto:joel@rtems.org" target="_blank">joel@rtems.org</a>> wrote:<br>
>> >> ><br>
>> >> > Hi<br>
>> >> ><br>
>> >> > There are a lot of Coverity issues related to device drivers which call mkdir("/dev") and ignore the return value. My first thought was that they should have (void) added since /dev could have been created by an earlier driver.<br>
>> >> ><br>
>> >> > Then it occurred to me that libcsupport/src/base_fs.c includes mkdir("/dev") and a fatal error if it cannot create it.<br>
>> >> ><br>
>> >> > Doesn't this mean that every call to mkdir("/dev") in a BSP or device driver is redundant? They should be removed since the base FS contents are always in place before any device drivers are called.<br>
>> >> ><br>
>> >> > Thoughts?<br>
>> >> ><br>
>> >><br>
>> >> Seems reasonable. You could probably add an assert that /dev exists.<br>
>> ><br>
>> ><br>
>> > The creation of the device node will fail if /dev does not exist so I think that<br>
>> > is covered.<br>
>> ><br>
>> > I think the call is pointless and changing it to stat() to make sure it exists<br>
>> > just adds bulk to the driver/BSP dependencies.<br>
>> ><br>
>> > I think just deleting it seems prudent. There is an error path if it doesn't<br>
>> > exist.<br>
>> ><br>
>><br>
>> ok. I was just suggesting conversion to an assert since that is some<br>
>> good practice when you have a belief/assumption but aren't totally<br>
>> convinced. It's fine with me either way. assert(mkdir("/dev") == -1)<br>
>> or just delete it.<br>
><br>
><br>
> Grrr.. I've looked again at the code and it is all Gaisler code doing something<br>
> like mkdir("/dev/leonXXX"). It really could fail. This should be a fatal error<br>
> and would seem to indicate that we need a grlib category of fatal BSP/driver<br>
> errors.<br>
<br>
Given the complexity of grlib, that would make some sense. Although<br>
maybe we can make it a little more generic... FATAL_THIRD_PARTY_DRIVER<br>
errors might be nicer on them, and can be reused<br></blockquote><div><br></div><div>Good suggestion. But I think this particular error is potentially even more generic.</div><div>Any device driver wanting to make a subdirectory /dev/XXX could generate this.<br>This seems like the right place to add this but I'm open for better names:<br><br>diff --git a/bsps/include/bsp/fatal.h b/bsps/include/bsp/fatal.h<br>index ec5902755e..0d8507e398 100644<br>--- a/bsps/include/bsp/fatal.h<br>+++ b/bsps/include/bsp/fatal.h<br>@@ -41,6 +41,7 @@ typedef enum {<br>   BSP_FATAL_CONSOLE_INSTALL_0,<br>   BSP_FATAL_CONSOLE_INSTALL_1,<br>   BSP_FATAL_CONSOLE_REGISTER_DEV_2,<br>+  BSP_FATAL_MAKE_SUBDIR_OF_DEV,<br> <br>   /* ARM fatal codes */<br>   BSP_ARM_A9MPCORE_FATAL_CLOCK_IRQ_INSTALL = BSP_FATAL_CODE_BLOCK(1),<br> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
>><br>
>><br>
>> > --joel<br>
>> >><br>
>> >><br>
>> >> > --joel<br>
>> >> > _______________________________________________<br>
>> >> > devel mailing list<br>
>> >> > <a href="mailto:devel@rtems.org" target="_blank">devel@rtems.org</a><br>
>> >> > <a href="http://lists.rtems.org/mailman/listinfo/devel" rel="noreferrer" target="_blank">http://lists.rtems.org/mailman/listinfo/devel</a><br>
</blockquote></div></div>