How would you implement the pilloried function in the Daily WTF?

The Daily WTF for 2008-11-28 pillories the following code:

static char *nice_num(long n)
{
    int neg = 0, d = 3;
    char *buffer = prtbuf;
    int bufsize = 20;

    if (n < 0)
    {
        neg = 1;
        n = -n;
    }
    buffer += bufsize;
    *--buffer = '\0';

    do
    {
        *--buffer = '0' + (n % 10);
        n /= 10;
        if (--d == 0)
        {
            d = 3;
            *--buffer = ',';
        }
    }
    while (n);

    if (*buffer == ',') ++buffer;
    if (neg) *--buffer = '-';
    return buffer;
}

How would you write it?

Answers


If you're a seasoned C programmer, you'll realize this code isn't actually that bad. It's relatively straightforward (for C), and it's blazingly fast. It has three problems:

  1. It fails on the edge case of LONG_MIN (-2,147,483,648), since negating this number produces itself in twos-complement
  2. It assumes 32-bit integers - for 64-bit longs, a 20-byte buffer is not big enough
  3. It's not thread-safe - it uses a global static buffer, so multiple threads calling it at the same time will result in a race condition

Problem #1 is easily solved with a special case. To address #2, I'd separate the code into two functions, one for 32-bit integers and one for 64-bit integers. #3 is a little harder - we have to change the interface to make completely thread-safe.

Here is my solution, based on this code but modified to address these problems:

static int nice_num(char *buffer, size_t len, int32_t n)
{
  int neg = 0, d = 3;
  char buf[16];
  size_t bufsize = sizeof(buf);
  char *pbuf = buf + bufsize;

  if(n < 0)
  {
    if(n == INT32_MIN)
    {
      strncpy(buffer, "-2,147,483,648", len);
      return len <= 14;
    }

    neg = 1;
    n = -n;
  }

  *--pbuf = '\0';

  do
  {
    *--pbuf = '0' + (n % 10);
    n /= 10;
    if(--d == 0)
    {
      d = 3;
      *--pbuf = ',';
    }
  }
  while(n > 0);

  if(*pbuf == ',') ++pbuf;
  if(neg) *--pbuf = '-';

  strncpy(buffer, pbuf, len);
  return len <= strlen(pbuf);
}

Explanation: it creates a local buffer on the stack and then fills that in in the same method as the initial code. Then, it copies it into a parameter passed into the function, making sure not to overflow the buffer. It also has a special case for INT32_MIN. The return value is 0 if the original buffer was large enough, or 1 if the buffer was too small and the resulting string was truncated.


Need Your Help

Using array_diff_assoc function in PHP

php arrays multidimensional-array associative-array

Hello I have two nested arrays and need to find the difference between the reference array and the data array. I am using array_dif_assoc function, and am unable to get the right difference, I am not

How to push UIViewController with replaced UINavigationController

iphone ios uiviewcontroller uinavigationcontroller uinavigationbar

I have a custom UINavigationController that draws a custom background. That navigation controller is already set when accessing self.navigationController on the root view controller. Now what I'd l...

About UNIX Resources Network

Original, collect and organize Developers related documents, information and materials, contains jQuery, Html, CSS, MySQL, .NET, ASP.NET, SQL, objective-c, iPhone, Ruby on Rails, C, SQL Server, Ruby, Arrays, Regex, ASP.NET MVC, WPF, XML, Ajax, DataBase, and so on.