[OpenWrt-Devel] base-files: For sysfixtime use hwclock if RTC available

From: Daniel Dickinson <openwrt@daniel.thecshore.com>

On systems that have an RTC prefer it to the file-based
time fixup (i.e. use hwclock when there is a permanent
clock instead of the faked up time logic that is needed
when there is not RTC).

Signed-off-by: Daniel Dickinson <openwrt@daniel.thecshore.com>
---
 package/base-files/files/etc/init.d/sysfixtime | 23 ++++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

On 10/01/2016 08:16, openwrt@daniel.thecshore.com wrote:
> +        else

whitespace error

> +		local curtime="$(date +%s)"
> +		lo

Hi,

On 10/01/16 06:42 AM, bittorf wireless )) Bastian Bittorf wrote:
>> +
>> +start() {
>> +	if [ -e /dev/rtc ]; then
>> +		hwclock -s
>
> please use the short form [ -e /dev/rtc ] && ...
>

Per private mail I've learned this is the current codebase standard, so 
will follow that, but the reason I tend to prefer the if..then is that 
if..then has the correct semantics when using set -e (that is causes 
termination on error, but not under normal operation) whereas [ xx ] && 
yy is not set -e safe and simply adding || true results in error 
conditions not being detected, so it is actually doing the wrong thing 
if you use set -e (unless the initial condition being false is an error 
and not just normal operation).

Regards,

Daniel

Actually I must have been smoking something when I thought I saw that 
problem (and no I don't actually).  I think it must have been in 
combination with some other error that I misremembered.

I just checked both bash and ash (and the docs) and they 'do the right 
thing'.

Regards,

Daniel

On 11/01/16 03:19 AM, Daniel Dickinson wrote:
> Hi,
>
> On 10/01/16 06:42 AM, bittorf wireless )) Bastian Bittorf wrote:
>>> +
>>> +start() {
>>> +    if [ -e /dev/rtc ]; then
>>> +        hwclock -s
>>
>> please use the short form [ -e /dev/rtc ] && ...
>>
>
> Per private mail I've learned this is the current codebase standard, so
> will follow that, but the reason I tend to prefer the if..then is that
> if..then has the correct semantics when using set -e (that is causes
> termination on error, but not under normal operation) whereas [ xx ] &&
> yy is not set -e safe and simply adding || true results in error
> conditions not being detected, so it is actually doing the wrong thing
> if you use set -e (unless the initial condition being false is an error
> and not just normal operation).
>
> Regards,
>
> Daniel
> _______________________________________________
> openwrt-devel mailing list
> openwrt-devel@lists.openwrt.org
> https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel
>

Hmmm...actually this may be a case of an old non-POSIX conformant shell 
I once had to work in.

Anyway any modern POSIX-compliant shell doesn't have this issue.

Regards,

Daniel

I did one other test I hadn't thought of originally:

#!/bin/sh

set -e

/bin/false && /bin/false

echo "not killed"

displays "not killed", so there still the issue that unlike if..then 
with set -e, && fails to exit on error condition (for the purposes of 
set -e it's like there is an implicit || /bin/true (really the exit 
status just gets ignored for an AND-OR list in POSIX terms)).

Regards,

Daniel

On 11/01/16 03:30 AM, Daniel Curran-Dickinson wrote:
> Actually I must have been smoking something when I thought I saw that
> problem (and no I don't actually).  I think it must have been in
> combination with some other error that I misremembered.
>
> I just check both bash and ash (and the docs) and they 'do the right
> thing'.
>
> Regards,
>
> Daniel
>
> On 11/01/16 03:19 AM, Daniel Dickinson wrote:
>> Hi,
>>
>> On 10/01/16 06:42 AM, bittorf wireless )) Bastian Bittorf wrote:
>>>> +
>>>> +start() {
>>>> +    if [ -e /dev/rtc ]; then
>>>> +        hwclock -s
>>>
>>> please use the short form [ -e /dev/rtc ] && ...
>>>
>>
>> Per private mail I've learned this is the current codebase standard, so
>> will follow that, but the reason I tend to prefer the if..then is that
>> if..then has the correct semantics when using set -e (that is causes
>> termination on error, but not under normal operation) whereas [ xx ] &&
>> yy is not set -e safe and simply adding || true results in error
>> conditions not being detected, so it is actually doing the wrong thing
>> if you use set -e (unless the initial condition being false is an error
>> and not just normal operation).
>>
>> Regards,
>>
>> Daniel
>> _______________________________________________
>> openwrt-devel mailing list
>> openwrt-devel@lists.openwrt.org
>> https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel
>>

Hi,

short summary for the impatient readers: the following text dives into the
subtile differences between errexit (set -e) and exit status in shell scripting


Am Mon, 11 Jan 2016 05:14:18 -0500
schrieb Daniel Dickinson <openwrt@daniel.thecshore.com>:

> I did one other test I hadn't thought of originally:
> 
> #!/bin/sh
> 
> set -e
> 
> /bin/false && /bin/false
> 
> echo "not killed"
> 
> displays "not killed", so there still the issue that unlike if..then 
> with set -e, && fails to exit on error condition (for the purposes of 
> set -e it's like there is an implicit || /bin/true (really the exit 
> status just gets ignored for an AND-OR list in POSIX terms)).

The dash manpage describes "-e" (errexit) as follows:
 The exit status of a command is considered to be explicitly tested if the
 command is used to control an if, elif, while, or until; or if the command is
 the left hand operand of an ``&&'' or ``||'' operator.
The bash manual explains the same - just a bit more lengty. Busybox's ash works
in the same way, as far as I noticed.

Thus the manual implies that the following two lines behave identical with
regards to errexit:

  if [ -e /somefile ]; then action; fi
  [ -e /somefile ] && action

In fact they do. If the test fails then action is not executed.
If the test succeeds then the action is executed. The exit status of action
determines the overall exit status of this line and thus may trigger errexit.

The subtile difference between the above two lines is their exit status.
The "if" variation returns the exit status of "action" (test succeeds) or zero
(test fails).
The "&&" variation returns the exit status of the last executed command. Thus a
failed test returns a non-zero exit status. But due to the definition of
errexit, only the exit status of the last item in a compound statement
may trigger an errexit event.

Thus both statements on their own will behave in the same way regarding errexit
even if their exit status may differ.

But there is a final catch: if these statements are the last ones in any
while/for/if/case block, then their exit status determines the exit status
of the whole block. Thus the surrounding while/for/if/case block may cause an
errexit.

Look at these examples:

= example A =
 for a in foo bar; do
   [ -e "$a" ] && echo "$a"
 done

= example B =
 for a in foo bar; do
   if [ -e "$a" ]; then echo "$a"; fi
 done

In example (A) you will see a failure (followed by errexit) if the file "bar"
does not exist. The state of "foo" is not relevant. This is a bit surprising.
In example (B) there will never be an error. This is probably in line with the
expectation of most users.

Thus it is advisable to add a "true" statement at the end of a while/for/if/case
block if last statement is a "||" or "&&" compound. This additional
statement does not cover any errors. It just works around the subtile
differences between "exit code" and "errexit triggering" in this specific
situation.

Lars