Re: bin/41945: calendar(1) doesn't recognize days of the week correctly sometimes

[Robert Elz <kre%munnari.OZ.AU@localhost>]

The following reply was made to PR bin/41945; it has been noted by GNATS.

From: Robert Elz <kre%munnari.OZ.AU@localhost>
To: Thomas Klausner <wiz%netbsd.org@localhost>
Cc: NetBSD bugtracking <gnats-bugs%netbsd.org@localhost>
Subject: Re: bin/41945: calendar(1) doesn't recognize days of the week 
correctly sometimes
Date: Wed, 18 Nov 2009 16:48:46 +0700

     Date:        Tue, 17 Nov 2009 16:41:00 +0100
     From:        Thomas Klausner <wiz%netbsd.org@localhost>
     Message-ID:  <20091117154100.GM10767%danbala.tuwien.ac.at@localhost>
 
   | calendar.c:330: warning: 'savech' may be used uninitialized in this 
function
 
 Oops - that one was me, attempting an optimisation, then not compiling
 with warnings on to notice I screwed up...
 
   | calendar.c:328: warning: 'val' may be used uninitialized in this function
 
 That's caused by the same thing, but is harmless, val can be used
 unintialised, but no-one cares, the flags field indicates whether val
 contains anything or not (and if it does, what kind of data is there),
 and the flags are only ever set after val has been set, so this one would
 be safe (ignoring theoretical unknown processors that trap on references
 to unit data ... which I tend to do).    But I have added an init of the
 variable just to make the warning go away.
 
 Try the appended patch instead (made against today's CVS version of
 calendar.c from the head - which is probably the same as it was last
 time, but I didn't check...)
 
 I also added a comment on the (very weird) FLDCHAR() macro, which takes
 an arg, that it never actually uses, but which still seems like the right
 way to write it...
 
 kre
 
 ps: I still suspect that discarding our (ancient) calendar and restarting
 with the current FreeBSD version would probably be a net win.
 
 --- /tmp/calendar.c    2009-11-18 16:35:43.000000000 +0700
 +++ calendar.c 2009-11-18 16:44:18.000000000 +0700
 @@ -63,6 +63,13 @@
  
  #include "pathnames.h"
  
 +      /* flags used by calendar file parser */
 +#define       F_ISMONTH       0x01
 +#define       F_ISDAY         0x02
 +#define       F_ISDOW         0x04
 +#define       F_WILDMONTH     0x10
 +#define       F_WILDDAY       0x20
 +
  static unsigned short lookahead = 1;
  static unsigned short weekend = 2;
  static char *fname = NULL;
 @@ -247,18 +254,13 @@
        int v1;
        int v2;
  
 -#define       F_ISMONTH       0x01
 -#define       F_ISDAY         0x02
 -#define F_WILDMONTH   0x04
 -#define F_WILDDAY     0x08
 -
        flags = 0;
  
        /* didn't recognize anything, skip it */
        if (!(v1 = getfield(endp, &endp, &flags)))
                return false;
  
 -      if (flags & F_ISDAY || v1 > 12) {
 +      if ((flags & (F_ISDAY|F_ISDOW)) || v1 > 12) {
                /* found a day */
                day = v1;
                /* if no recognizable month, assume wildcard ('*') month */
 @@ -295,10 +297,17 @@
        if (flags & F_WILDMONTH && flags & F_ISDAY && day == tp->tm_mday)
                return true;
  
 +      if (flags & F_WILDMONTH && flags & F_ISDOW && day == tp->tm_wday + 1)
 +              return true;
 +
        if (flags & F_ISMONTH && flags & F_WILDDAY && month == tp->tm_mon + 1)
                return true;
  
 -      if (flags & F_ISDAY)
 +      if (flags & F_ISMONTH && flags & F_ISDOW && month == tp->tm_mon + 1 &&
 +          day == tp->tm_wday + 1)
 +              return true;
 +
 +      if (flags & F_ISDOW)
                day = tp->tm_mday + (((day - 1) - tp->tm_wday + 7) % 7);
        day = cumdays[month] + day;
  
 @@ -320,9 +329,15 @@
        char *start;
        char savech;
  
 +/*
 + * note this macro has an arg that isn't used ... it is retained
 + * (it is believed) to make the macro call look more "natural"
 + * and suggest at the call site what is happening.
 + */
  #define FLDCHAR(a) (*p != '\0' && !isdigit((unsigned char)*p) && \
      !isalpha((unsigned char)*p) && *p != '*')
  
 +      val = 0;
        for (/*EMPTY*/; FLDCHAR(*p); ++p)
                continue;
        if (*p == '*') {                        /* `*' is current month */
 @@ -343,17 +358,21 @@
                *endp = p;
                return val;
        }
 -      for (start = p; *p != '\0' && isalpha((unsigned char)*++p); /*EMPTY*/)
 +      for (start = p; *p != '\0' && isalpha((unsigned char)*p); p++)
                continue;
 +
        savech = *p;
 -      *p = '\0';
 -      if ((val = getmonth(start)) != 0)
 -              *flags |= F_ISMONTH;
 -      else if ((val = getday(start)) != 0)
 -              *flags |= F_ISDAY;
 -      else {
 -              *p = savech;
 -              return 0;
 +      if (p != start) {
 +              *p = '\0';
 +              if ((val = getmonth(start)) != 0)
 +                      *flags |= F_ISMONTH;
 +              else if ((val = getday(start)) != 0)
 +                      *flags |= F_ISDOW;
 +              else {
 +                      *p = savech;
 +                      *endp = start;
 +                      return 0;
 +              }
        }
        for (*p = savech; FLDCHAR(*p); ++p)
                continue;