Consider adding chsprintf

[jgobuyan]

Hi,

I ported cJSON code to run under ChibiOS and one of the things I needed was a lightweight sprintf() function in order to avoid linking in the standard one. I essentially modified the existing chprintf.c/chprintf.h files to add a chsprintf() function because you already have all the supporting static functions. I essentially replicated the chprintf() function but there may be a way to combine them into one function somehow.

Code: Select all

@@ -260,4 +260,180 @@ void chprintf(BaseSequentialStream *chp, const char *fmt, ...) {
   }
 }
 
+/**
+ * @brief   Formatted string function.
+ * @details This function implements a minimal @p sprintf() like functionality
+ *          with output to a string.
+ *          The general parameters format is: %[-][width|*][.precision|*][l|L]p.
+ *          The following parameter types (p) are supported:
+ *          - <b>x</b> hexadecimal integer.
+ *          - <b>X</b> hexadecimal long.
+ *          - <b>o</b> octal integer.
+ *          - <b>O</b> octal long.
+ *          - <b>d</b> decimal signed integer.
+ *          - <b>D</b> decimal signed long.
+ *          - <b>u</b> decimal unsigned integer.
+ *          - <b>U</b> decimal unsigned long.
+ *          - <b>c</b> character.
+ *          - <b>s</b> string.
+ *          .
+ *
+ * @param[in] buf       pointer to a string buffer
+ * @param[in] fmt       formatting string
+ */
+void chsprintf(char *buf, const char *fmt, ...) {
+  va_list ap;
+  char *p, *s, c, filler;
+  int i, precision, width;
+  bool_t is_long, left_align;
+  long l;
+#if CHPRINTF_USE_FLOAT
+  float f;
+  char tmpbuf[2*MAX_FILLER + 1];
+#else
+  char tmpbuf[MAX_FILLER + 1];
+#endif
+
+  va_start(ap, fmt);
+  while (TRUE) {
+    c = *fmt++;
+    if (c == 0) {
+      va_end(ap);
+      *(buf++) = 0;
+      return;
+    }
+    if (c != '%') {
+      *(buf++) = (uint8_t)c;
+      continue;
+    }
+    p = tmpbuf;
+    s = tmpbuf;
+    left_align = FALSE;
+    if (*fmt == '-') {
+      fmt++;
+      left_align = TRUE;
+    }
+    filler = ' ';
+    if (*fmt == '.') {
+      fmt++;
+      filler = '0';
+    }
+    width = 0;
+    while (TRUE) {
+      c = *fmt++;
+      if (c >= '0' && c <= '9')
+        c -= '0';
+      else if (c == '*')
+        c = va_arg(ap, int);
+      else
+        break;
+      width = width * 10 + c;
+    }
+    precision = 0;
+    if (c == '.') {
+      while (TRUE) {
+        c = *fmt++;
+        if (c >= '0' && c <= '9')
+          c -= '0';
+        else if (c == '*')
+          c = va_arg(ap, int);
+        else
+          break;
+        precision *= 10;
+        precision += c;
+      }
+    }
+    /* Long modifier.*/
+    if (c == 'l' || c == 'L') {
+      is_long = TRUE;
+      if (*fmt)
+        c = *fmt++;
+    }
+    else
+      is_long = (c >= 'A') && (c <= 'Z');
+
+    /* Command decoding.*/
+    switch (c) {
+    case 'c':
+      filler = ' ';
+      *p++ = va_arg(ap, int);
+      break;
+    case 's':
+      filler = ' ';
+      if ((s = va_arg(ap, char *)) == 0)
+        s = "(null)";
+      if (precision == 0)
+        precision = 32767;
+      for (p = s; *p && (--precision >= 0); p++)
+        ;
+      break;
+    case 'D':
+    case 'd':
+    case 'I':
+    case 'i':
+      if (is_long)
+        l = va_arg(ap, long);
+      else
+        l = va_arg(ap, int);
+      if (l < 0) {
+        *p++ = '-';
+        l = -l;
+      }
+      p = ltoa(p, l, 10);
+      break;
+#if CHPRINTF_USE_FLOAT
+    case 'f':
+      f = (float) va_arg(ap, double);
+      if (f < 0) {
+        *p++ = '-';
+        f = -f;
+      }
+      p = ftoa(p, f);
+      break;
+#endif
+    case 'X':
+    case 'x':
+      c = 16;
+      goto unsigned_common;
+    case 'U':
+    case 'u':
+      c = 10;
+      goto unsigned_common;
+    case 'O':
+    case 'o':
+      c = 8;
+unsigned_common:
+      if (is_long)
+        l = va_arg(ap, unsigned long);
+      else
+        l = va_arg(ap, unsigned int);
+      p = ltoa(p, l, c);
+      break;
+    default:
+      *p++ = c;
+      break;
+    }
+    i = (int)(p - s);
+    if ((width -= i) < 0)
+      width = 0;
+    if (left_align == FALSE)
+      width = -width;
+    if (width < 0) {
+      if (*s == '-' && filler == '0') {
+        *(buf++) = (uint8_t)*s++;
+        i--;
+      }
+      do
+        *(buf++) = (uint8_t)filler;
+      while (++width != 0);
+    }
+    while (--i >= 0)
+      *(buf++) =  (uint8_t)*s++;
+
+    while (width) {
+      *(buf++) =  (uint8_t)filler;
+      width--;
+    }
+  }
+}
 /** @} */

Would you consider adding it in?

Jerome

[Tectu]

I didn't take any look to your implementation at all. However, I really like the idea since I use sprintf() myself very often. It would definitely be nice if it would be in the mainline. To avoid unwanted memory usage, it could be inlcuded on his own like chprintf() does currently as well.


~ Tectu

[Giovanni]

Potentially chprintf() already does that, it writes on a generic stream, you just need to create a stream wrapping a string buffer and use chprintf() on that.

memstreams.c does that however I haven't tested it for a while.

Giovanni

[jgobuyan]

What I did was simply copy chprintf() and paste to chsprintf() and replace all references to chSequentialStreamPut() to *(buf++). The original chprintf() is there as well. It's just a little inefficient to have the code duplicated, but it was a quick and dirty fix for me. I think rewriting chprintf() to be something more like vprintf() would allow a common function to be used for both.

I think an exact equivalent of the standard sprintf() would be more useful for porting existing code rather than one that goes through the memstreams abstraction.

Jerome

[colin]

Just remember how dangerous sprintf() is... on the other hand, snprintf() is safe, since it won't overflow your buffer. You can write safe code with sprintf() (as long as you don't include %s format specifier...) if you are very careful.

But I really like Giovanni's idea of using a "stream buffer" that actually writes to a string in memory, then it can do the buffer overrun checking for you as well. It might be a tiny bit less efficient than an sprintf() that has "*str++ = c;" in line, but I bet that would be minimal and it would be safe too!

[jgobuyan]

I would normally avoid using sprintf in embedded code but I'm just porting existing code (cJSON) that expects an sprintf like function. I don't how easy it is to modify it to use the memstreams abstraction.

[colin]

I would normally avoid using sprintf in embedded code but I'm just porting existing code (cJSON) that expects an sprintf like function. I don't how easy it is to modify it to use the memstreams abstraction.

Actually what I propose is a simple sprintf() / snprintf() (preferably the latter!!) should be implemented using the existing streams -- i.e. all that has to be written is the simple wrapper function with the sprintf() API that calls chprintf() with the appropriate memory stream.