[PATCH v2] pwdgrp.c: removed assert causing runtime issues

[Joel Sherrill]

On Thu, Mar 18, 2021 at 2:14 PM Gedare Bloom <gedare at rtems.org> wrote:

> On Thu, Mar 18, 2021 at 11:40 AM Ryan Long <ryan.long at 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.

>
> >    /*
> >     *  Initialize /etc/passwd
> > --
> > 1.8.3.1
> >
> > _______________________________________________
> > devel mailing list
> > devel at rtems.org
> > http://lists.rtems.org/mailman/listinfo/devel
> _______________________________________________
> devel mailing list
> devel at rtems.org
> http://lists.rtems.org/mailman/listinfo/devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rtems.org/pipermail/devel/attachments/20210318/4c8bf25c/attachment.html>