[Top][All Lists]

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

Re: new module 'system-quote'

From: Bruno Haible
Subject: Re: new module 'system-quote'
Date: Thu, 10 May 2012 00:07:18 +0200
User-agent: KMail/4.7.4 (Linux/3.1.10-1.9-desktop; KDE/4.7.4; x86_64; ; )

Hi Paul,

> Shouldn't that be system_quote_length (interpreter, string) + 1?

Oops, corrected.

> > # define CMD_FORBIDDEN_CHARS "\n\r"
> That macro is never used -- is that intended?

I'm not sure what to do with strings that contain such characters.
Surely, rejecting them would be better than silent truncation of the
command line. But system_quote* API is not made for returning error

> Are there restrictions about what can go into the strings?
> For example, can they contain UTF-8 encoding errors?

I believe that the strings need to be correctly encoded in the locale
encoding (which can be GB2312 or Big5, for example), for both
because the Windows XP machine I have access to has only 8-bit locale

Btw, Windows does not have a locale in UTF-8 encoding.

> shell_quote_argv and system_quote_argv are essentially the same
> function, differing only in that one invokes
> shell_quote_length/shell_quote_copy and the other calls
> system_quote_length/system_quote_copy.  Perhaps they should be
> refactored into calling a single function that is parameterized by
> quoting function.

I agree. The code has now reached a size where the code duplication
makes future changes risky. I'll refactor as you say.

> This refactoring would be easier if we changed the
> signature of system_quote_copy to be this:
>    size_t system_quote_copy (char *p, const char *string);
> so that it returns the length of the string that it creates, and it
> doesn't store anything if !P.  That way, we need just one quoting
> function, which we can pass as an argument to the refactored function
> mentioned above.  E.g., instead of this:
>    size_t length = system_quote_length (interpreter, str) + 2;
>    char *buf = xmalloc (length);
>    char *p = system_quote_copy (buf, interpreter, str);
>    strcpy (p, "\n");
> one would write this:
>    size_t length = system_quote_copy (NULL, interpreter, str) + 2;
>    char *buf = xmalloc (length);
>    char *p = buf + system_quote_copy (buf, interpreter, str);
>    strcpy (p, "\n");

But I don't agree here. I don't find that this set of calling conventions
is easier to remember for the user (except if he's being using wcstombs
all the day).


reply via email to

[Prev in Thread] Current Thread [Next in Thread]