> On Jun 25, 2020, at 8:39 AM, Kevin J. McCarthy <[email protected]> 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
