[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [Libvir] [PATCH] Enhance virBuffer code



Jim Meyering wrote:
"Richard W.M. Jones" <rjones redhat com> wrote:

Jim Meyering wrote:
"Richard W.M. Jones" <rjones redhat com> wrote:
Jim Meyering wrote:
What do you think of using this?

  isascii (*p) && isalnum (*p)
I'm not sure I'm qualified to say what this does on EBCDIC, but quite
likely lots of other code breaks there too anyway.  This is nicely
self-documenting anyway.
As Daniel suggested, isalnum is locale-sensitive.
If there's a locale with an alphabetic byte that is outside
the logical a-zA-Z range, yet still within 0..127, then the above
expression will give a false-positive for that byte.

I've been inclined to stop worrying about EBCDIC for years, but a quick
search on the web finds that people are still stuck using it, and do
report bugs in ASCII-assuming code.

This is why autoconf goes to the trouble of doing this:
  tr abcdefghijklmnopqrstuvwxyz ABCDEFGHIJKLMNOPQRSTUVWXYZ
not this:
  tr a-z A-Z
to convert to upper case.
Another factor to consider here is that it doesn't matter if this
function over-escapes, but it does matter if the function
under-escapes. That is to say, it could escape every character as a
%xx hex code, which would be ugly and slightly inefficient but not
wrong.

IMHO, if you don't use the all-enumerating switch-based code that Daniel
objects to, it'd be good to document (in both loops) that the test is
inaccurate with EBCDIC, and explain why it's ok to get false positives.

Without comments, people might be tempted to use a similar test in a
context where the differences matter.

OK, how about this?

Rich.

+    for (p = str; *p; ++p) {
+ /* Want to escape only A-Z and 0-9. This may not work on EBCDIC. */
+        if (isascii (*p) && isalnum (*p))
+            grow_size++;
+        else
+            grow_size += 3; /* %ab */
+    }
+
+    if (virBufferGrow (buf, grow_size) == -1)
+        return -1;
+
+    for (p = str; *p; ++p) {
+       /* See comment above. */
+        if (isascii (*p) && isalnum (*p))
+            buf->content[buf->use++] = *p;
+        else {
+            uc = (unsigned char) *p;
+            buf->content[buf->use++] = '%';
+            buf->content[buf->use++] = hex[uc >> 4];
+            buf->content[buf->use++] = hex[uc & 0xf];
+        }
+    }


--
Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/
Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod
Street, Windsor, Berkshire, SL4 1TE, United Kingdom.  Registered in
England and Wales under Company Registration No. 03798903

Attachment: smime.p7s
Description: S/MIME Cryptographic Signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]