4
\$\begingroup\$

I am working on a compiler that does a pretty heavy work on strings in order to get them correctly displayed. So far I have an algorithm that transforms a given string like (In plaintext):

"This is a \"string\""

After matching the complete token I get something like the following (C string):

"This is a \\\"string\\\""

And the same goes for hexadecimal literals and common escape characters (\r \n \t \b), the code I've came is this one:

#include <stdint.h>

int is_hex(int c) {
  return (is_digit(c) || (c >= 'a' && c <= 'f') || (c >= 'A' && c <= 'F'));
}

static char get_nibble(char T) {
  if (T >= 'A' && T <= 'F')
    return (T - 'a') + 10;
  else if (T >= 'a' && T <= 'f')
    return (T - 'A') + 10;
  return (T - '0');
}

char *post_string(char *in, uint32_t *len, const char **err) {
  if (!in || !len)
    return NULL;

  char *out = in, *tmp = in; uint32_t _len = 0; const char *_err = NULL;
  char esc = 0;
  while (in && *in && !_err) {
    if (*in == '\\') {
      ++in;
      switch (*in) {
        case '"': esc = '"'; ++in; break;
        case 'r': esc = '\r'; ++in; break;
        case 'n': esc = '\n'; ++in; break;
        case 't': esc = '\t'; ++in; break;
        case 'b': esc = '\b'; ++in; break;
        case 'x':
        case 'X': {
          ++in;
          if (!is_hex(*in) || !is_hex(in[1]))
            _err = "Invalid character on hexadecimal escape.";
          if (_err)
            break;
          esc = ((get_nibble(*in) << 4) | (get_nibble(in[1]))) & 0xff; in += 2;
        } break;
        case '\'': esc = '\''; ++in; break;
        case '\\': esc = '\\'; ++in; break;
        case 0:
        default:
          _err = "Unexpected '\\' with no escape sequence."; break;
      }
      if (!_err)
        tmp[_len] = esc, ++_len;
    }
    else {
      tmp[_len] = *in; ++in, ++_len;
    }
  }

  out[_len] = 0;  /* Set the end of string. */
  *len = _len;    /* Set the final string length. */
  if (err)        /* Set error message back. */
    *err = _err;
  return out;
}

On above implementation I did make sure that a null terminated string is passed to the implementation.

There are no requirements but would be great if no allocation gets done on this step, also a standard implementation should be achieved (Not sure if mine is standard though).

Below is a test main:

#include <stdlib.h>

int main(void) {
  char t[] = "This is a \\\"string\\\"";
  uint32_t len = 0;
  printf("Before: %s\n", t);
  post_string(t, &len, NULL);
  printf("After : %s\n", t);
  return 0;
}

How can I improve this code?

\$\endgroup\$
  • \$\begingroup\$ Are you intentionally ignoring octal escapes? Also \a, \v, \f, \" \u and \U? \$\endgroup\$ – Toby Speight May 15 '18 at 17:21
  • \$\begingroup\$ @TobySpeight \" is already handled, Unicode support will come later. I just need ASCII characters for the moment, but didn't know about \a or \f. \$\endgroup\$ – NaCl May 15 '18 at 18:04
  • 1
    \$\begingroup\$ Sorry, I missed the \" (spotted it when writing my answer). Here's a full list! \$\endgroup\$ – Toby Speight May 15 '18 at 18:07
5
\$\begingroup\$

Use <cctype> for character classification

Instead of defining is_hex() (and depending on abcdef and ABCDEF being contiguously coded), better to use the standard isxdigit().

get_nibble makes a similar assumption - it's arguably safer to use a switch for this.

On the whole, the logic looks good. I'm not a fan of names beginning with _ - even if you do fully understand the rules about reserved identifiers, many of your readers will not!

Some expressions can be simplified with post-increment. Consider this fragment:

if (!_err)
    tmp[_len] = esc, ++_len;
} else {
    tmp[_len] = *in; ++in, ++_len;
}

That could be

if (!_err)
    tmp[_len++] = esc;
} else {
    tmp[_len++] = *in++;
}

That's fairly idiomatic usage, and should be familiar to anyone who's looked at strcpy or the like. In fact, slightly more idiomatic would be to use a pointer, rather than indexing:

char *post_string(char *const in, size_t *len, const char **err) {
    if (!in || !len)
        return NULL;

    char *out = in, *p = in;
    const char *int_err = NULL;

    while (*p && !int_err) {
        if (*p != '\\') {
            /* normal case */
            *out++ = *p++;
        } else {
            /* escape sequence */
            switch (*++p) {
            case 'a': *out++ = '\a'; ++p; break;
            case 'b': *out++ = '\b'; ++p; break;
            case 'f': *out++ = '\f'; ++p; break;
            case 'n': *out++ = '\n'; ++p; break;
            case 'r': *out++ = '\r'; ++p; break;
            case 't': *out++ = '\t'; ++p; break;
            case 'v': *out++ = '\v'; ++p; break;

            case '"': case '\'': case '\\': *out++ = *p++; case '?': break;

            case 'x': case 'X':
                if (!isxdigit(p[1]) || !isxdigit(p[2])) {
                    int_err = "Invalid character on hexadecimal escape.";
                } else {
                    *out++ = (char)(get_nibble(p[1]) * 0x10 + get_nibble(p[2]));
                    p += 3;
                }
                break;

            default:
                int_err = "Unexpected '\\' with no escape sequence."; break;
            }
        }
    }

    *out = '\0';                /* Set the end of string. */
    *len = p - in;              /* Set the final string length. */
    if (int_err && err)         /* Set error message back. */
        *err = int_err;
    return in;
}

Testing

Some of the code appears completely untested (and I've fixed above). In particular, passing NULL as the err parameter - as in the sample main() - will provoke undefined behaviour if your string is invalid).


Modified code

I've also added code to accept 1-3 octal digits, and modified the hexadecimal reader to accept 1+ digits, and included tests of each error branch.

#include <ctype.h>
#include <limits.h>
#include <stddef.h>

static unsigned int get_nibble(char c)
{
    switch(c) {
        case '0': return 0;
        case '1': return 1;
        case '2': return 2;
        case '3': return 3;
        case '4': return 4;
        case '5': return 5;
        case '6': return 6;
        case '7': return 7;
        case '8': return 8;
        case '9': return 9;
        case 'A': case 'a': return 0xa;
        case 'B': case 'b': return 0xb;
        case 'C': case 'c': return 0xc;
        case 'D': case 'd': return 0xd;
        case 'E': case 'e': return 0xe;
        case 'F': case 'f': return 0xf;
    }
    /* shouldn't happen */
    return 0;
}

char *post_string(char *const in, size_t *len, const char **err) {
    if (!in || !len)
        return NULL;

    char *out = in, *p = in;
    const char *int_err = NULL;

    while (*p && !int_err) {
        if (*p != '\\') {
            /* normal case */
            *out++ = *p++;
        } else {
            /* escape sequence */
            switch (*++p) {
            case 'a': *out++ = '\a'; ++p; break;
            case 'b': *out++ = '\b'; ++p; break;
            case 'f': *out++ = '\f'; ++p; break;
            case 'n': *out++ = '\n'; ++p; break;
            case 'r': *out++ = '\r'; ++p; break;
            case 't': *out++ = '\t'; ++p; break;
            case 'v': *out++ = '\v'; ++p; break;

            case '"': case '\'': case '\\': case '?': *out++ = *p++; break;

            case 'x': case 'X':
                if (!isxdigit(*++p)) {
                    int_err = "Invalid character on hexadecimal escape."; break;
                } else {
                    unsigned int x = 0;  /* assuming int is larger than char */
                    while (isxdigit(*p)) {
                        x *= 0x10;
                        x += get_nibble(*p++);
                        if (x > UCHAR_MAX) {
                            int_err = "Out of range hexadecimal escape."; break;
                        }
                    }
                    *out++ = (char)x; break;
                }

            default:
                if (isdigit(*p) && *p < '8') {
                    /* octal escape */
                    unsigned int x = 0;  /* assuming int is larger than char */
                    for (int i = 0;  i < 3 && isdigit(*p) && *p < '8';  ++i) {
                        x = x * 010 + (unsigned)*p++ - '0';
                    }
                    if (x > UCHAR_MAX) {
                        int_err = "Out of range octal escape."; break;
                    }
                    *out++ = (char)x; break;
                } else {
                    int_err = "Unexpected '\\' with no escape sequence."; break;
                }
            }
        }
    }

    *out = '\0';                /* Set the end of string. */
    *len = (size_t)(p - in);    /* Set the final string length. */
    if (int_err && err)         /* Set error message back. */
        *err = int_err;
    return in;
}


#include <stdio.h>
#include <stdlib.h>
#include <string.h>

int main(void) {
  char t[256] = "This is a \\\"string\\\" with \\x43 \\X063 hex\\"
      "nnewline\\ttab\\t\\?\\'\\\"\\\\octal\\132\\538 \\0null";
  size_t len = 0;
  printf("Before: %s\n", t);
  post_string(t, &len, NULL);
  printf("After : %s\n\n", t);

  const char *err = NULL;
  strcpy(t, "This has a hex error: \\xg");
  printf("Before: %s\n", t);
  post_string(t, &len, &err);
  printf("Error: %s\n\n", err);

  strcpy(t, "Hex beyond ASCII: \\xfff");
  printf("Before: %s\n", t);
  post_string(t, &len, &err);
  printf("Error: %s\n\n", err);

  strcpy(t, "Octal beyond ASCII: \\777");
  printf("Before: %s\n", t);
  post_string(t, &len, &err);
  printf("Error: %s\n\n", err);

  strcpy(t, "not an escape: \\9");
  printf("Before: %s\n", t);
  post_string(t, &len, &err);
  printf("Error: %s\n\n", err);

  strcpy(t, "backslash at end: \\");
  printf("Before: %s\n", t);
  post_string(t, &len, &err);
  printf("Error: %s\n\n", err);
}
\$\endgroup\$
  • \$\begingroup\$ Why the char *const as first argument? \$\endgroup\$ – NaCl May 15 '18 at 18:13
  • \$\begingroup\$ underscore-uppercase-anything, and underscore-underscore-anything is reserved, underscore-lowercase-anything is fine. \$\endgroup\$ – Deduplicator May 16 '18 at 0:18
  • \$\begingroup\$ @NaCl - just to avoid any accidents, and ensure that p-in at the end would still be correct. Note that the const here isn't part of the function signature - it only constrains the implementation (in particular, it's very different from char const *). \$\endgroup\$ – Toby Speight May 16 '18 at 7:46
  • \$\begingroup\$ @chux - fixed that. \$\endgroup\$ – Toby Speight May 16 '18 at 8:27
1
\$\begingroup\$

How can I improve this code?

Hexadecimal escape sequences

OP's code assumes a hexadecimal escape sequence has exactly 2 hex characters. If code wants to follow the C standard:

These sequences are at least 1 hex digit. There is no specified upper bound to the number of digits, yet "The value of an octal or hexadecimal escape sequence shall be in the range of representable values for the corresponding type:" (unsigned char) C11 §6.4.4.4 9

    case 'X': // Upper case X is not specified to be valid here, but is a common extension.
    case 'x': {
      bool found = false;
      unsigned esc_h = 0;
      while (isxdigit((unsigned char) in[1]) {
         found = true;
         in++;
         unsigned xdigit = get_nibble(*in);
         if (esc_h >= UCHAR_MAX/16 && 
             (esc_h > UCHAR_MAX/16 || xdigit > UCHAR_MAX%16)) {
           // escape value too big and UB
           _err = "Out of range hexadecimal escape.";
           break;
         }
         esc_h = esc_h * 16 + xdigit;
      }
      if (!found) {
        _err = "Invalid character on hexadecimal escape.";
        break;
      }
      esc = (char) esc_h;

Octal escape sequences

OP omitted these yet there are quite common in the case of \0. These are either 1, 2 or 3 octal digits after a \. Easy enough to code. Only values > UCHAR_MAX are troublesome.

Embedded null character

Converting "\x00Hello World" results in what appears to be an empty string "". Perhaps return the length to indicate the length of converted characters.

Use size_t

size_t is the best type for array indexing and sizing, neither too narrow nor too wide.

// char *post_string(char *in, uint32_t *len, const char **err) {
char *post_string(char *in, size_t *len, const char **err) {

Single escape character alternative

Perhaps not faster, yet another approach

// C11 6.4.4.4  There are  11 of them
static const char *esc_chars = "abtnvfn\'\"\?\\";
static const char *esc_vals = "\a\b\t\n\v\f\n\'\"\?\\";

char *esc_ptr = strchr(esc_chars, *in);
if (esc_prr) {
  if (*esc_ptr) break;  // null character detected 
  int offset = esc_ptr - esc_chars;
  esc = esc_vals[offset];
  }
else {
  ...
} 
\$\endgroup\$
  • \$\begingroup\$ Well, by standard I meant that the code provided in your answer should follow the standard, my bad if any assumption was made :p I'm working with ASCII encoded characters, so I wanted to 'enforce' users to use at least 1 hex char and not more than 2, because of the encoding, tough... For the embedded nullchar, that is not a problem but I'll keep it in mind! \$\endgroup\$ – NaCl May 15 '18 at 23:49
  • \$\begingroup\$ @NaCl To be clear, your code does not "use at least 1 hex char and not more than 2". Instead it requires 2. \$\endgroup\$ – chux - Reinstate Monica May 16 '18 at 3:18

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.