xiaoxiang781216 commented on code in PR #6029:
URL: https://github.com/apache/incubator-nuttx/pull/6029#discussion_r848202084


##########
libs/libc/time/lib_strptime.c:
##########
@@ -53,382 +67,556 @@ static const struct {
     const char *d_fmt;
     const char *t_fmt;
     const char *t_fmt_ampm;
-} _DefaultTimeLocale = {
-    {
-        "Sun","Mon","Tue","Wed","Thu","Fri","Sat",
-    },
-    {
-        "Sunday", "Monday", "Tuesday", "Wednesday", "Thursday",
-        "Friday", "Saturday"
-    },
-    {
-        "Jan", "Feb", "Mar", "Apr", "May", "Jun",
-        "Jul", "Aug", "Sep", "Oct", "Nov", "Dec"
-    },
-    {
-        "January", "February", "March", "April", "May", "June", "July",
-        "August", "September", "October", "November", "December"
-    },
-    {
-        "AM", "PM"
-    },
-    "%a %b %d %H:%M:%S %Y",
-    "%m/%d/%y",
-    "%H:%M:%S",
-    "%I:%M:%S %p"
+} g_defaulttimelocale =
+{
+  {
+    "Sun", "Mon", "Tue", "Wed", "Thu", "Fri", "Sat",
+  },
+  {
+    "Sunday", "Monday", "Tuesday", "Wednesday", "Thursday",
+    "Friday", "Saturday"
+  },
+  {
+    "Jan", "Feb", "Mar", "Apr", "May", "Jun",
+    "Jul", "Aug", "Sep", "Oct", "Nov", "Dec"
+  },
+  {
+    "January", "February", "March", "April", "May", "June", "July",
+    "August", "September", "October", "November", "December"
+  },
+  {
+    "AM", "PM"
+  },
+  "%a %b %d %H:%M:%S %Y",
+  "%m/%d/%y",
+  "%H:%M:%S",
+  "%I:%M:%S %p"
 };
 
-#define _ctloc(x) (_DefaultTimeLocale.x)
+struct century_relyear
+{
+  int century;
+  int relyear;
+};
 
-/*
- * We do not implement alternate representations. However, we always
- * check whether a given modifier is allowed for a certain conversion.
- */
-#define _ALT_E          0x01
-#define _ALT_O          0x02
-#define _LEGAL_ALT(x)       { if (alt_format & ~(x)) return (0); }
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
 
+static int _conv_num(FAR const unsigned char **buf, FAR int *dest,
+                     int llim, int ulim)
+{
+  int result = 0;
+  int rulim = ulim;
 
-struct century_relyear {
-    int century;
-    int relyear;
-};
-static  int _conv_num(const unsigned char **, int *, int, int);
-static  unsigned char *_strptime(const unsigned char *, const char *, struct 
tm *,
-        struct century_relyear *);
+  while (isspace(**buf))
+    {
+      (*buf)++;
+    }
 
+  if (**buf < '0' || **buf > '9')
+    {
+      return 0;
+    }
 
-char *
-strptime(const char *buf, const char *fmt, struct tm *tm)
-{
-    struct century_relyear cr;
-    cr.century = TM_YEAR_BASE;
-    cr.relyear = -1;
-    return (char*)(_strptime((const unsigned char*)buf, fmt, tm, &cr));
+  /* we use rulim to break out of the loop when we run out of digits */
+
+  do
+    {
+      result *= 10;
+      result += *(*buf)++ - '0';
+      rulim /= 10;
+    }
+  while ((result * 10 <= ulim) && rulim && **buf >= '0' && **buf <= '9');
+
+  if (result < llim || result > ulim)
+    {
+      return 0;
+    }
+
+  *dest = result;
+  return 1;
 }
 
-static unsigned char *
-_strptime(const unsigned char *buf, const char *fmt, struct tm *tm, struct 
century_relyear *cr)
+static FAR unsigned char *_strptime(FAR const unsigned char *buf,

Review Comment:
   change to:
   ```
   static FAR const unsigned char *_strptime(...
   ```



##########
libs/libc/time/lib_strptime.c:
##########
@@ -53,382 +67,556 @@ static const struct {
     const char *d_fmt;
     const char *t_fmt;
     const char *t_fmt_ampm;
-} _DefaultTimeLocale = {
-    {
-        "Sun","Mon","Tue","Wed","Thu","Fri","Sat",
-    },
-    {
-        "Sunday", "Monday", "Tuesday", "Wednesday", "Thursday",
-        "Friday", "Saturday"
-    },
-    {
-        "Jan", "Feb", "Mar", "Apr", "May", "Jun",
-        "Jul", "Aug", "Sep", "Oct", "Nov", "Dec"
-    },
-    {
-        "January", "February", "March", "April", "May", "June", "July",
-        "August", "September", "October", "November", "December"
-    },
-    {
-        "AM", "PM"
-    },
-    "%a %b %d %H:%M:%S %Y",
-    "%m/%d/%y",
-    "%H:%M:%S",
-    "%I:%M:%S %p"
+} g_defaulttimelocale =
+{
+  {
+    "Sun", "Mon", "Tue", "Wed", "Thu", "Fri", "Sat",
+  },
+  {
+    "Sunday", "Monday", "Tuesday", "Wednesday", "Thursday",
+    "Friday", "Saturday"
+  },
+  {
+    "Jan", "Feb", "Mar", "Apr", "May", "Jun",
+    "Jul", "Aug", "Sep", "Oct", "Nov", "Dec"
+  },
+  {
+    "January", "February", "March", "April", "May", "June", "July",
+    "August", "September", "October", "November", "December"
+  },
+  {
+    "AM", "PM"
+  },
+  "%a %b %d %H:%M:%S %Y",
+  "%m/%d/%y",
+  "%H:%M:%S",
+  "%I:%M:%S %p"
 };
 
-#define _ctloc(x) (_DefaultTimeLocale.x)
+struct century_relyear
+{
+  int century;
+  int relyear;
+};
 
-/*
- * We do not implement alternate representations. However, we always
- * check whether a given modifier is allowed for a certain conversion.
- */
-#define _ALT_E          0x01
-#define _ALT_O          0x02
-#define _LEGAL_ALT(x)       { if (alt_format & ~(x)) return (0); }
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
 
+static int _conv_num(FAR const unsigned char **buf, FAR int *dest,
+                     int llim, int ulim)
+{
+  int result = 0;
+  int rulim = ulim;
 
-struct century_relyear {
-    int century;
-    int relyear;
-};
-static  int _conv_num(const unsigned char **, int *, int, int);
-static  unsigned char *_strptime(const unsigned char *, const char *, struct 
tm *,
-        struct century_relyear *);
+  while (isspace(**buf))
+    {
+      (*buf)++;
+    }
 
+  if (**buf < '0' || **buf > '9')
+    {
+      return 0;
+    }
 
-char *
-strptime(const char *buf, const char *fmt, struct tm *tm)
-{
-    struct century_relyear cr;
-    cr.century = TM_YEAR_BASE;
-    cr.relyear = -1;
-    return (char*)(_strptime((const unsigned char*)buf, fmt, tm, &cr));
+  /* we use rulim to break out of the loop when we run out of digits */
+
+  do
+    {
+      result *= 10;
+      result += *(*buf)++ - '0';
+      rulim /= 10;
+    }
+  while ((result * 10 <= ulim) && rulim && **buf >= '0' && **buf <= '9');
+
+  if (result < llim || result > ulim)
+    {
+      return 0;
+    }
+
+  *dest = result;
+  return 1;
 }
 
-static unsigned char *
-_strptime(const unsigned char *buf, const char *fmt, struct tm *tm, struct 
century_relyear *cr)
+static FAR unsigned char *_strptime(FAR const unsigned char *buf,
+                                    FAR const char *fmt,
+                                    FAR struct tm *tm,
+                                    FAR struct century_relyear *cr)
 {
-    unsigned char c;
-    const unsigned char *bp;
-    size_t len = 0;
-    int alt_format, i;
-
-    bp = (unsigned char *)buf;
-    while ((c = *fmt) != '\0') {
-        /* Clear `alternate' modifier prior to new conversion. */
-        alt_format = 0;
-
-        /* Eat up white-space. */
-        if (isspace(c)) {
-            while (isspace(*bp))
-                bp++;
+  unsigned char c;
+  FAR const unsigned char *bp;
+  size_t len = 0;
+  int alt_format;
+  int i;
+
+  bp = (FAR const unsigned char *)buf;
+  while ((c = *fmt) != '\0')
+    {
+      /* Clear `alternate' modifier prior to new conversion. */
+
+      alt_format = 0;
+
+      /* Eat up white-space. */
+
+      if (isspace(c))
+        {
+          while (isspace(*bp))
+            bp++;
+
+          fmt++;
+          continue;
+        }
 
-            fmt++;
-            continue;
+      if ((c = *fmt++) != '%')
+        {
+          goto literal;
         }
 
-        if ((c = *fmt++) != '%')
-            goto literal;
+    again:
+      switch (c = *fmt++)
+        {
+          case '%': /* "%%" is converted to "%". */
+          literal:
+            if (c != *bp++)
+              {
+                return NULL;
+              }
 
-again:      switch (c = *fmt++) {
-        case '%':   /* "%%" is converted to "%". */
-literal:
-        if (c != *bp++)
-            return (NULL);
+            break;
 
-        break;
+            /* "Alternative" modifiers. Just set the appropriate flag
+            * and start over again.
+            */
 
-        /*
-         * "Alternative" modifiers. Just set the appropriate flag
-         * and start over again.
-         */
-        case 'E':   /* "%E?" alternative conversion modifier. */
+          case 'E': /* "%E?" alternative conversion modifier. */
             _LEGAL_ALT(0);
             alt_format |= _ALT_E;
             goto again;
 
-        case 'O':   /* "%O?" alternative conversion modifier. */
+          case 'O': /* "%O?" alternative conversion modifier. */
             _LEGAL_ALT(0);
             alt_format |= _ALT_O;
             goto again;
 
-        /*
-         * "Complex" conversion rules, implemented through recursion.
-         */
-        case 'c':   /* Date and time, using the locale's format. */
+            /* "Complex" conversion rules, implemented through recursion. */
+
+          case 'c': /* Date and time, using the locale's format. */
             _LEGAL_ALT(_ALT_E);
             if (!(bp = _strptime(bp, _ctloc(d_t_fmt), tm, cr)))
-                return (NULL);
+              {
+                return NULL;
+              }
+
             break;
 
-        case 'D':   /* The date as "%m/%d/%y". */
+          case 'D': /* The date as "%m/%d/%y". */
             _LEGAL_ALT(0);
             if (!(bp = _strptime(bp, "%m/%d/%y", tm, cr)))
-                return (NULL);
+              {
+                return NULL;
+              }
+
             break;
 
-        case 'R':   /* The time as "%H:%M". */
+          case 'R': /* The time as "%H:%M". */
             _LEGAL_ALT(0);
             if (!(bp = _strptime(bp, "%H:%M", tm, cr)))
-                return (NULL);
+              {
+                return NULL;
+              }
+
             break;
 
-        case 'r':   /* The time as "%I:%M:%S %p". */
+          case 'r': /* The time as "%I:%M:%S %p". */
             _LEGAL_ALT(0);
             if (!(bp = _strptime(bp, "%I:%M:%S %p", tm, cr)))
-                return (NULL);
+              {
+                return NULL;
+              }
+
             break;
 
-        case 'T':   /* The time as "%H:%M:%S". */
+          case 'T': /* The time as "%H:%M:%S". */
             _LEGAL_ALT(0);
             if (!(bp = _strptime(bp, "%H:%M:%S", tm, cr)))
-                return (NULL);
+              {
+                return NULL;
+              }
+
             break;
 
-        case 'X':   /* The time, using the locale's format. */
+          case 'X': /* The time, using the locale's format. */
             _LEGAL_ALT(_ALT_E);
             if (!(bp = _strptime(bp, _ctloc(t_fmt), tm, cr)))
-                return (NULL);
+              {
+                return NULL;
+              }
+
             break;
 
-        case 'x':   /* The date, using the locale's format. */
+          case 'x': /* The date, using the locale's format. */
             _LEGAL_ALT(_ALT_E);
             if (!(bp = _strptime(bp, _ctloc(d_fmt), tm, cr)))
-                return (NULL);
+              {
+                return NULL;
+              }
+
             break;
 
-        /*
-         * "Elementary" conversion rules.
-         */
-        case 'A':   /* The day of week, using the locale's form. */
-        case 'a':
+            /* "Elementary" conversion rules. */
+
+          case 'A': /* The day of week, using the locale's form. */
+          case 'a':
             _LEGAL_ALT(0);
-            for (i = 0; i < 7; i++) {
+            for (i = 0; i < 7; i++)
+              {
                 /* Full name. */
+
                 len = strlen(_ctloc(day[i]));
-                if (strncasecmp(_ctloc(day[i]), (const char*)bp, len) == 0)
-                    break;
+                if (strncasecmp(_ctloc(day[i]),
+                                (const char *)bp, len) == 0)
+                  break;
 
                 /* Abbreviated name. */
+
                 len = strlen(_ctloc(abday[i]));
-                if (strncasecmp(_ctloc(abday[i]), (const char*)bp, len) == 0)
-                    break;
-            }
+                if (strncasecmp(_ctloc(abday[i]),
+                                (const char *)bp, len) == 0)
+                  break;
+              }
 
             /* Nothing matched. */
+
             if (i == 7)
-                return (NULL);
+              {
+                return NULL;
+              }
 
             tm->tm_wday = i;
             bp += len;
             break;
 
-        case 'B':   /* The month, using the locale's form. */
-        case 'b':
-        case 'h':
+          case 'B': /* The month, using the locale's form. */
+          case 'b':
+          case 'h':
             _LEGAL_ALT(0);
-            for (i = 0; i < 12; i++) {
+            for (i = 0; i < 12; i++)
+              {
                 /* Full name. */
+
                 len = strlen(_ctloc(mon[i]));
-                if (strncasecmp(_ctloc(mon[i]), (const char*)bp, len) == 0)
+                if (strncasecmp(_ctloc(mon[i]),
+                                (const char *)bp, len) == 0)
+                  {
                     break;
+                  }
 
                 /* Abbreviated name. */
+
                 len = strlen(_ctloc(abmon[i]));
-                if (strncasecmp(_ctloc(abmon[i]), (const char*)bp, len) == 0)
+                if (strncasecmp(_ctloc(abmon[i]),
+                                (const char *)bp, len) == 0)
+                  {
                     break;
-            }
+                  }
+              }
 
             /* Nothing matched. */
+
             if (i == 12)
-                return (NULL);
+              {
+                return NULL;
+              }
 
             tm->tm_mon = i;
             bp += len;
             break;
 
-        case 'C':   /* The century number. */
+          case 'C': /* The century number. */
             _LEGAL_ALT(_ALT_E);
             if (!(_conv_num(&bp, &i, 0, 99)))
+              {
                 return (NULL);
+              }
 
             cr->century = i * 100;
             break;
 
-        case 'd':   /* The day of month. */
-        case 'e':
+          case 'd': /* The day of month. */
+          case 'e':
             _LEGAL_ALT(_ALT_O);
             if (!(_conv_num(&bp, &tm->tm_mday, 1, 31)))
+              {
                 return (NULL);
+              }
+
             break;
 
-        case 'k':   /* The hour (24-hour clock representation). */
+          case 'k': /* The hour (24-hour clock representation). */
             _LEGAL_ALT(0);
+
             /* FALLTHROUGH */
-        case 'H':
+
+          case 'H':
             _LEGAL_ALT(_ALT_O);
             if (!(_conv_num(&bp, &tm->tm_hour, 0, 23)))
-                return (NULL);
+              {
+                return NULL;
+              }
+
             break;
 
-        case 'l':   /* The hour (12-hour clock representation). */
+          case 'l': /* The hour (12-hour clock representation). */
             _LEGAL_ALT(0);
+
             /* FALLTHROUGH */
-        case 'I':
+
+          case 'I':
             _LEGAL_ALT(_ALT_O);
             if (!(_conv_num(&bp, &tm->tm_hour, 1, 12)))
+              {
                 return (NULL);
+              }
+
             break;
 
-        case 'j':   /* The day of year. */
+          case 'j': /* The day of year. */
             _LEGAL_ALT(0);
             if (!(_conv_num(&bp, &tm->tm_yday, 1, 366)))
-                return (NULL);
+              {
+                return NULL;
+              }
+
             tm->tm_yday--;
             break;
 
-        case 'M':   /* The minute. */
+          case 'M': /* The minute. */
             _LEGAL_ALT(_ALT_O);
             if (!(_conv_num(&bp, &tm->tm_min, 0, 59)))
-                return (NULL);
+              {
+                return NULL;
+              }
+
             break;
 
-        case 'm':   /* The month. */
+          case 'm': /* The month. */
             _LEGAL_ALT(_ALT_O);
             if (!(_conv_num(&bp, &tm->tm_mon, 1, 12)))
-                return (NULL);
+              {
+                return NULL;
+              }
+
             tm->tm_mon--;
             break;
 
-        case 'p':   /* The locale's equivalent of AM/PM. */
+          case 'p': /* The locale's equivalent of AM/PM. */
             _LEGAL_ALT(0);
+
             /* AM? */
+
             len = strlen(_ctloc(am_pm[0]));
-            if (strncasecmp(_ctloc(am_pm[0]), (const char*)bp, len) == 0) {
-                if (tm->tm_hour > 12)   /* i.e., 13:00 AM ?! */
-                    return (NULL);
+            if (strncasecmp(_ctloc(am_pm[0]), (const char *)bp, len) == 0)
+              {
+                if (tm->tm_hour > 12) /* i.e., 13:00 AM ?! */
+                  {
+                    return NULL;
+                  }
                 else if (tm->tm_hour == 12)
+                  {
                     tm->tm_hour = 0;
+                  }
 
                 bp += len;
                 break;
-            }
+              }
+
             /* PM? */
+
             len = strlen(_ctloc(am_pm[1]));
-            if (strncasecmp(_ctloc(am_pm[1]), (const char*)bp, len) == 0) {
-                if (tm->tm_hour > 12)   /* i.e., 13:00 PM ?! */
-                    return (NULL);
+            if (strncasecmp(_ctloc(am_pm[1]), (const char *)bp, len) == 0)
+              {
+                if (tm->tm_hour > 12) /* i.e., 13:00 PM ?! */
+                  {
+                    return NULL;
+                  }
                 else if (tm->tm_hour < 12)
+                  {
                     tm->tm_hour += 12;
+                  }
 
                 bp += len;
                 break;
-            }
+              }
 
             /* Nothing matched. */
-            return (NULL);
 
-        case 'S':   /* The seconds. */
+            return NULL;
+
+          case 'S': /* The seconds. */
             _LEGAL_ALT(_ALT_O);
             if (!(_conv_num(&bp, &tm->tm_sec, 0, 61)))
-                return (NULL);
+              {
+                return NULL;
+              }
+
             break;
 
-        case 'U':   /* The week of year, beginning on sunday. */
-        case 'W':   /* The week of year, beginning on monday. */
+          case 'U': /* The week of year, beginning on sunday. */
+          case 'W': /* The week of year, beginning on monday. */
             _LEGAL_ALT(_ALT_O);
-            /*
-             * XXX This is bogus, as we can not assume any valid
-             * information present in the tm structure at this
-             * point to calculate a real value, so just check the
-             * range for now.
-             */
-             if (!(_conv_num(&bp, &i, 0, 53)))
-                return (NULL);
-             break;
+            /* XXX This is bogus, as we can not assume any valid
+            * information present in the tm structure at this
+            * point to calculate a real value, so just check the
+            * range for now.
+            */
+
+            if (!(_conv_num(&bp, &i, 0, 53)))
+              {
+                return NULL;
+              }
 
-        case 'w':   /* The day of week, beginning on sunday. */
+            break;
+
+          case 'w': /* The day of week, beginning on sunday. */
             _LEGAL_ALT(_ALT_O);
             if (!(_conv_num(&bp, &tm->tm_wday, 0, 6)))
-                return (NULL);
+              {
+                return NULL;
+              }
+
             break;
 
-        case 'Y':   /* The year. */
+          case 'Y': /* The year. */
             _LEGAL_ALT(_ALT_E);
             if (!(_conv_num(&bp, &i, 0, 9999)))
-                return (NULL);
+              {
+                return NULL;
+              }
 
             cr->relyear = -1;
             tm->tm_year = i - TM_YEAR_BASE;
             break;
 
-        case 'y':   /* The year within the century (2 digits). */
+          case 'y': /* The year within the century (2 digits). */
             _LEGAL_ALT(_ALT_E | _ALT_O);
             if (!(_conv_num(&bp, &cr->relyear, 0, 99)))
-                return (NULL);
+              {
+                return NULL;
+              }
+
             break;
 
-        /*
-         * Miscellaneous conversions.
-         */
-        case 'n':   /* Any kind of white-space. */
-        case 't':
+            /* Miscellaneous conversions. */
+
+          case 'n': /* Any kind of white-space. */
+          case 't':
             _LEGAL_ALT(0);
             while (isspace(*bp))
+              {
                 bp++;
-            break;
+              }
 
+            break;
 
-        default:    /* Unknown/unsupported conversion. */
-            return (NULL);
+          default: /* Unknown/unsupported conversion. */
+            return NULL;
         }
-
-
     }
 
-    /*
-     * We need to evaluate the two digit year spec (%y)
-     * last as we can get a century spec (%C) at any time.
-     */
-    if (cr->relyear != -1) {
-        if (cr->century == TM_YEAR_BASE) {
-            if (cr->relyear <= 68)
-                tm->tm_year = cr->relyear + 2000 - TM_YEAR_BASE;
-            else
-                tm->tm_year = cr->relyear + 1900 - TM_YEAR_BASE;
-        } else {
-            tm->tm_year = cr->relyear + cr->century - TM_YEAR_BASE;
+  /* We need to evaluate the two digit year spec (%y)
+   * last as we can get a century spec (%C) at any time.
+   */
+
+  if (cr->relyear != -1)
+  {
+    if (cr->century == TM_YEAR_BASE)
+    {
+      if (cr->relyear <= 68)
+        {
+          tm->tm_year = cr->relyear + 2000 - TM_YEAR_BASE;
+        }
+      else
+        {
+          tm->tm_year = cr->relyear + 1900 - TM_YEAR_BASE;
         }
     }
+    else
+    {
+      tm->tm_year = cr->relyear + cr->century - TM_YEAR_BASE;
+    }
+  }
 
-    return (unsigned char*)bp;
+  return (FAR unsigned char *)bp;

Review Comment:
   remove the cast



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscr...@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to