> On Jun 25, 2020, at 8:39 AM, Kevin J. McCarthy <ke...@8t8.us> wrote:
> 
> I wonder why they treated \ and " differently though: it appears purposely 
> done.

I'd guess just this: for an incoming \ or " you still store the incoming 
character merely preceding it with a \, whereas for the other incoming 
characters you store a different character, e.g., incoming tab becomes t. 
Really the entire while loop in escape_string() could be simplified (if you 
consider the repeated macro expansion code being inserted in each case block), 
completely eliminating the switch and macro, per something like the following:

if (src)
  while (p - dst < len && *src) {
    static const char* escape = "\n\r\t\\"", escaped="nrt\\"";
    const char* e = strchr(escape, *src);
    if (e) {*p++ = '\\'; if (p - dst < len); *p++ = escaped[e - escape]; }
    else *p++ = *src;
    }

But then to further think about the code in this function, the

  if (!len)
    return 0;

at the top is questionable. As far as I can tell, the rest of the code implies 
these semantics:

 - I will store at most 'len' bytes
 - I will always put a \0 as my last stored byte
 - I will return the length of the stored string (not including the \0)

So when a zero is returned, does this mean "hey there wasn't enough room to 
store anything at all, not even store a \0", or does it mean "hey you gave me 
sufficient room to store stuff and I correctly processed an incoming string of 
length 0 and therefore stored just a \0" ?

At this moment, the two places that call escape_string() will never pass in a 
len of 0. In pretty_var() if the destination runs out of room save for the spot 
to hold the final \0 by the time escape_string() is called then a 1 is passed 
in.

Consequently, it seems the top of escape_string() ought to do something like 
throw a fatal error if the incoming len is 0, perhaps only do that check in a 
debug build, i.e., make it an assertion.

--
Alex

Reply via email to