[oi-dev] Mistake in svc:/network/install:default script?

[Irek Szczesniak]

On Fri, Nov 29, 2013 at 4:59 PM, Jim Klimov <jimklimov at cos.ru> wrote:
> Hello,
>
>   While reviewing the service code for system initialization,
> I stumbled upon this in /lib/svc/method/net-install:
>
>
>                                 if [ "$intf" == "$NET_V4IF" ]; then
>                                         net_process_v4_pg
>                                 else
>                                         net_process_v6_pg
>                                 fi
>                                 if [ $? -ne $SMF_EXIT_OK ]; then
>                                         #
>                                         # Last error wins.
>                                         #
>                                         ecode=$?
>                                         errs=`expr $errs + 1`
>                                 else
>                                         ifcnt=`expr $ifcnt + 1`
>                                 fi
>
> The routine sets "ecode" (error code) which would be returned
> upon exit from net_process_install(). I believe that the clause
> in "Last error wins" is wrong, and would use "$?" value from
> the test "[" invokation, which would be a successful zero.
> But I am not certain about how this should be processed in the
> logic, that is - should such errors be treated as failures of
> the service or not.
>
> If yes (errors are failures), the exit code should be non-zero
> via a temporary variable ($res) like this:
>
>                                 if [ "$intf" == "$NET_V4IF" ]; then
>                                         net_process_v4_pg
>                                 else
>                                         net_process_v6_pg
>                                 fi
>                                 res=$?
>                                 if [ $res -ne $SMF_EXIT_OK ]; then
>                                         #
>                                         # Last error wins.
>                                         #
>                                         ecode=$res
>                                         errs=`expr $errs + 1`
>                                 else
>                                         ifcnt=`expr $ifcnt + 1`
>                                 fi
>
>
> Does it make sense?

I think so, but statements like ifcnt=`expr $ifcnt + 1` make me vomit
at the same time. I think it had been established that POSIX
statements are a good idea for readability and performance, so ((
ifcnt++ )) or (( ifcnt=ifcnt+1 )) would be better than calling expr(1)
for simple arithmetic

Likewise if [ $res -ne $SMF_EXIT_OK ]; can be simplified to if (( res
!= SMF_EXIT_OK ));

Irek