12:11 p.m.
On Friday 31 October 2014 18:18:14 mzatko(a)redhat.com wrote:
+#include <config.h>
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <memory.h>
memory.h is not needed here.
+#include <c-ctype.h>
Prefer the "..." form for it (comes with gnulib).
+#include "rl.h"
+
+static char program_name[] = "fish";
This is provided by including guestfs-internal-frontend.h, and you'll
need guestfs.h prior including the former; so:
#include "guestfs.h"
#include "guestfs-internal-frontend.h"
+int
+hexdigit (char d)
+{
+ switch (d) {
+ case '0'...'9': return d - '0';
+ case 'a'...'f': return d - 'a' + 10;
+ case 'A'...'F': return d - 'A' + 10;
+ default: return -1;
+ }
+}
+
+// backslash unescape for readline
Prefer comments in the /* ... */ format, so:
/* Backslash unescape for readline. */
+char *
+bs_unescape_filename (char *str)
If "str" is not changed, just make it const, so callers know that.
+{
+ char *p = calloc(strlen(str) + 1, 1);
+ strcpy(p, str);
Sounds like strdup? It is POSIX.1-2007, and available also earlier;
in case not, gnulib has a strdup module.
+ error:
+ fprintf (stderr, ("%s: invalid escape sequence in string
(starting at offset %d)\n"),
+ program_name, (int) (p - start));
The message above needs gettext, so _(...).
+// backslash scape
Ditto (for comment style).
+char *
+bs_escape_filename (char *p)
Ditto (for constness of parameter).
+{
+ char *start = p;
+ // four times original length - if all chars are unprintable
+ // new string would be \xXY\xWZ
Ditto (for comment style).
+ char *n = calloc(strlen(p) * 4 + 1, 1);
I'm not sure calloc'ing the whole buffer is needed here; just create
it as usual with malloc, write on it, and make sure to set the last
character as '\0'.
+ char *nstart = n;
+ if (strlen(p) == 0) {
+ n[0] = '\0';
+ return n;
+ }
This optimization for empty strings would be better to be done before
allocating the resulting buffer, something like:
if (p[0] == '\0')
return strdup ("");
+ switch (*p) {
+ case '\\': break;
Shouldn't \ be escaped as well?
+ case '\a': *(n++) = '\\'; *n = 'a';
break;
+ case '\b': *(n++) = '\\'; *n = 'b'; break;
+ case '\f': *(n++) = '\\'; *n = 'f'; break;
+ case '\n': *(n++) = '\\'; *n = 'n'; break;
+ case '\r': *(n++) = '\\'; *n = 'r'; break;
+ case '\t': *(n++) = '\\'; *n = 't'; break;
+ case '\v': *(n++) = '\\'; *n = 'v'; break;
+ case '"': *(n++) = '\\'; *n = '"'; break;
+ case '\'': *(n++) = '\\'; *n = '\''; break;
+ case '?': *(n++) = '\\'; *n = '?'; break;
+ case ' ': *(n++) = '\\'; *n = ' '; break;
+
+ default:
+ // Hexadecimal escape unprintable character. This violates identity
+ // after composition of bsquote_filename after debsquote_filename
+ // (i.e. can escape some characters differently).
Ditto (for comment style).
Also, the names mentioned here don't seem the one used for the functions.
+ if (!c_isprint(*p)) {
+ n += sprintf(n, "\\x%x", (int) (*p & 0xff)) - 1;
Check the return of sprintf here.
> + } else {
> + *n = *p;
> + }
> + break;
+ error:
+ fprintf (stderr, ("%s: invalid escape sequence in string
(starting at offset %d)\n"),
+ program_name, (int) (p - start));
Ditto (for gettext).
+ nstart = realloc(nstart, strlen(nstart));
This is done to shrink the nstart buffer? I don't think it's needed,
especially if it is properly '\0'-ended.
+#ifndef _RL_H
This include guard seems a bit generic, please use something a bit more
specific like FISH_RL_H.
+#ifdef __cplusplus
+extern "C" {
+#endif
There is no C++ code in libguestfs, so this is not needed.
--
Pino Toscano