On Thu, Mar 18, 2021 at 3:11 PM Chris Johns <chr...@rtems.org> wrote:
> On 19/3/21 6:19 am, Joel Sherrill wrote: > > On Thu, Mar 18, 2021 at 2:14 PM Gedare Bloom <ged...@rtems.org > > <mailto:ged...@rtems.org>> wrote: > > On Thu, Mar 18, 2021 at 11:40 AM Ryan Long <ryan.l...@oarcorp.com > > <mailto:ryan.l...@oarcorp.com>> wrote: > > > > > > Changed the _Assert_unused_variable_equals macro to just a (void) > due to > > > /etc having already been created by the network stack > initialization > > > or an initial filesystem image. > > > > > > Updates #4282 > > > --- > > > cpukit/libcsupport/src/pwdgrp.c | 9 ++++----- > > > 1 file changed, 4 insertions(+), 5 deletions(-) > > > > > > diff --git a/cpukit/libcsupport/src/pwdgrp.c > b/cpukit/libcsupport/src/pwdgrp.c > > > index f4a10f7..edc8aff 100644 > > > --- a/cpukit/libcsupport/src/pwdgrp.c > > > +++ b/cpukit/libcsupport/src/pwdgrp.c > > > @@ -36,7 +36,6 @@ > > > #include <stdint.h> > > > > > > #include <rtems/seterr.h> > > > -#include <rtems/score/assert.h> > > > > > > #include "pwdgrp.h" > > > > > > @@ -63,13 +62,13 @@ static void init_file(const char *name, const > char > > *content) > > > */ > > > static void pwdgrp_init(void) > > > { > > > - int sc; > > > - > > > /* > > > * Do the best to create this directory. > > > + * > > > + * /etc could be created by the network stack initialization or > an initial > > > + * filesystem image. Deliberately ignore the return value. > > > */ > > > - sc = mkdir("/etc", S_IRWXU | S_IRGRP | S_IXGRP | S_IROTH | > S_IXOTH); > > > - _Assert_Unused_variable_equals(sc, 0); > > > + (void) mkdir("/etc", S_IRWXU | S_IRGRP | S_IXGRP | S_IROTH | > S_IXOTH); > > > > > > > So this seems fine, but it got me thinking, does this issue affect > all > > the other mkdir-related patches recently done? > > > > Probably, we should use > > _Assert(sc == 0 || errno == EEXIST); > > (void) sc; > > > > If we are sure that it is OK for it to exist and that's the errno > returned. :) > > > > But if it isn't, that's probably another bug. > > > > FWIW this seems typical of static analysis issues to me. FIx it wrong, > then > > fix it again a correct way, and then realize there was an even better > way to > > fix it. I presented a case at the Flight Software Workshop where I think > I went > > through multiple solutions before realizing B0 was special in POSIX to > hang > > up a line and the UART device driver had to say "not supported" instead > of > > an assert to avoid a divide by zero. > > > > These are often more subtle than they appear on first examination. > > This patch flip flop smells of deeper architecture issues. Ryan is doing a > great > job in handling these tough converity issues and he or anyone else can > never > know what is happening else where because nothing is specified. Every > place /etc > is needed has to add code to handle this and that is wrong. No where in > RTEMS do > we tolerate this type of coding. > The grlib case was quite unique to them. It wasn't creating /dev as I recall, it was creating a directory under /dev as I understood the code. This is one of them as it is on the 5 branch. strcpy(priv->prefix, "/dev/rastatmtc0"); priv->prefix[14] += dev->minor_drv; mkdir(priv->prefix, S_IRWXU | S_IRWXG | S_IRWXO); priv->prefix[15] = '/'; priv->prefix[16] = '\0'; > > I think I said before the real solution is to add a SYSINIT or what ever > that > creates "the RTEMS" root fs. We document it and make is standard. We make > the > initialise point something drivers and code can depend on and then these > make > dirs etc can be stripped away. > I'd tend to say a sysinit method for /etc. This will impact the libbsd code but it would be better to put it in one place. No point in adding it to every configuration even if not needed. /dev is already there in sysinit because it is already part of the base filesystem image. I don't see a second mkdir() on just it. src/base_fs.c: rv = mkdir( "/dev", S_IRWXU | S_IRGRP | S_IXGRP | S_IROTH | S_IXOTH ); > > Chris >
_______________________________________________ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
