Hi Steven, Thanks for your response. It appears to me that I have offended you with my last message. Should this be the case, I do apologise, this was not my intention at all.
On Sun, 2006-07-30 at 19:34 -0400, Steven M. Robbins wrote: > Quoting Andree Leidenfrost <[EMAIL PROTECTED]>: > > > Steven, Bruno, > > > > Attached please find a patch and demo program for a new function > > mr_stresc() to properly escape strings for use as arguments with (the > > likes of) system() and popen(). > > > > I have thought about using functions like exec() or fork() to avoid > > system() and popen(). I don't really see how the two latter would be > > generally evil. > > I never said system() is generally evil and I did not mean to imply > that. Both system() and exec() have their uses, but you need to know > *all* the rules in order to use them. The common pitfall when using > system() is that you need to escape the string because it will be > interpreted by the shell. Using exec() is one way to avoid that > pitfall. Doing proper escaping is another, perfectly reasonable > solution. > > > To the contrary, e.g. using a function that submits > > things to 'sh -c' means we have a sane environment like a PATH and so > > forth. > > Yeah, well ... that depends on whether you can presume the user does > have a sane PATH variable. I'm inclined to believe the opposite, > actually. Interesting territory we are entering here me thinks. Why would you be inclined to say that something as fundamental as the PATH variable can not be assumed to be sane? > > I therefore suggest use of the attached function when calling system() > > or popen() where required. I believe this is low-risk, low-overhead, > > little work, a clean approach and can be done bit by bit. > > > > What do you say? > > Proper escaping is not impossible, but it is pretty hard due to the > arcane syntax of the shell. However, your previous message implied > that the filenames are always passed inside double quotes, and > therefore there are exactly three characters that need escaping. Good point! Quoting using double-quotes is indeed an assumption that I do make. I should have explicitly said so. (The only other quoting alternative, i.e. using single quotes is a no-go as it would completely rule out the use of ' in file names.) > Since you have a very restricted escaping problem, I agree that > escaping is a much easier solution that replacing system() by > fork()/exec(). Ok. > However, your proposed mr_stresc() function has two flaws: > > 1. New memory is allocated each time so you run the risk of a memory > leak if the return value is not freed in the caller (and, indeed, it > is not in the mondo patch you attach). Thank you for pointing this out! My assumption that nesting the call to mr_stresc() inside sprintf() makes it so that the memory is freed when the outside function, i.e. sprintf() finishes is obviously wrong as I've also verified with valgrind now. To address this I define a variable to store the result of mr_stresc() and explicitly free it in the attached revised version. > 2. Not enough memory is allocated so you're going to overrun the > buffer anytime there is a character to escape. Have a closer look at > the manpage for strspn(). [Note: I find remarks like 'Have a closer look...' unhelpful.] So strspn() is not what we want, fair enough. Apparently, there is no other standard function that is either, so I use a modified version of the while loop in the revised version instead. I you have a constructive suggestion what could be used instead, I'd be absolutely interested. Please find a revised version of the demo program with the two above changes attached. Again, comments more than welcome! (The actual mondo patch would be changed accordingly as well of course.) > > -Steve Cheers, Andree -- Andree Leidenfrost @ Debian Developer Sydney - Australia
#include <stdlib.h>
#include <string.h>
#include <stdio.h>
char *mr_stresc(const char *instr, const char *toesc, const char escchr) {
char *inptr = NULL;
char *retstr = NULL;
char *retptr = NULL;
char *escptr = NULL;
int cnt = 0;
inptr = (char *)instr;
while (*inptr != '\0') {
escptr = (char *)toesc;
while (*escptr != '\0') {
if (*inptr == *escptr++) {
cnt++;
break;
}
}
*inptr++;
}
inptr = (char *)instr;
retstr = (char *)malloc(strlen(inptr) + cnt + 1);
retptr = (char *)retstr;
while (*inptr != '\0') {
escptr = (char *)toesc;
while (*escptr != '\0') {
if (*inptr == *escptr++) {
*retptr++ = escchr;
break;
}
}
*retptr++ = *inptr++;
}
*retptr = '\0';
return retstr;
}
int main() {
const char escchr = '\\';
const char escape_list[3] = "`$\\";
char string[44] = "These need escaping: `$\\, these don't: abc.";
char *result;
printf("Before: %s\n", string);
result = mr_stresc(string, escape_list, escchr);
printf("After: %s\n", result);
free(result);
return 0;
}
signature.asc
Description: This is a digitally signed message part
