[Toybox] [PATCH] Fix several bugs in date.

Rob Landley rob at landley.net
Mon Aug 10 23:58:52 PDT 2015


On 08/10/2015 10:39 PM, enh wrote:
> Fix several bugs in date.

Sigh. I honestly did test it, although clearly not thoroughly enough.
(That's how I set my clock to the year 2157, which networkmangler did
_not_ like...)

Ow. I see I checked in debug code. (I put my debug code flush with the
left edge so it's easy to spot and yank again.) Sorry about that.

> We were exiting before actually setting the date, chkmktime was always
> failing, and we would segfault in glibc's strftime for non-Unix dates.
> Plus one of the tests was mangled.

Regression test suite. This is another one where I need to run it in a
VM with root access...

> Also fix existing tests and add some more, and stop using the overly
> vague "bad" in errors.

I intentionally use "bad" because I'm trying to restrict the error
message vocabulary so non-native english speakers have a fighting chance
to follow along without a dictionary. See "Error messages and
internationalization" in http://landley.net/toybox/design.html

It's likely localized man pages are available, or posix standards, or a
translation of toybox's help -ah output web page. But annotating a bunch
of strings with _("thingy") in each command is something I'd prefer to
avoid because it's intrusive and waaaay too easy to introduce printf
bugs. (And half the command names are english words anyway so a certain
amount of english is probably unavoidable, but I would like to minimize
the requirement for the 2/3 of the planet it's inconvenient for.)

I had two messages this time because failure to handle input and failure
to handle output are two distinct error conditions. (Failure to parse
date is because it's a bad date. Failure to _set_ the date is usually a
permissions issue, you'll note the "cannot set date" and "bad format"
messages are perror_exit() and the libc message probably _is_
internationalized.)

Generally when I want to provide more details about failure to parse a
string I say "bad stringname" and then give the string and the offset in
it, ala sed's error_exit("bad pattern '%s'@%ld (%c)", errstart,
line-errstart+1L, *line);

Other times I use "bad $ADJECTIVE date" where you can treat $ADJECTIVE
as a swear word and still get the general idea. You're distinguishing
failure to parse ("bad date format", and since format is already a word
in the help text it's not _entirely_ new) from the date itself being
invalid ("bad date").

Rather than asking non english speakers to distinguish "invalid" from
"bad", I'd rather try to print the input and output dates to show why
they don't match, ala error_msg("%s != %s", unfiltered, filtered);

I checked in about 1/3 of your patch (most obvious bugfixes), I need to
look at the rest in the morning. (I wanted to turn chmktime() into
xmktime() and stick it in lib/xwrap.c but it didn't error_exit().
Changing the semantics of that makes life easier there. But not
something to fiddle with at 2am... :)

Thanks,

Rob

 1439276332.0


More information about the Toybox mailing list