Salut Bruno, On Wed, 2006-08-02 at 02:41 +0200, Bruno Cornec wrote: > Hello, > > Andree Leidenfrost said on Mon, Jul 31, 2006 at 09:37:47PM +1000: > > > > > 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? > > Well, I would tend to also agree on the fact mondo should avoid trusting > too much users, when it doesn't make sense. > > That's why I think we should use a config file to include all the > commands used by mondo, and provide the full path name to these > commands. > That fixes which one is used (and should be LSB/FSH compliant) and allow > for some exotic distro to change that conf file only to make it work. > Definitively for 3.0.x
I am certainly happy for you to make that design decision. I only
caution that it is a balancing act. If we define too many things
statically in configuration we might end up in some sort of maintenance
hell where we constantly have to adjust configuration to all sorts of
(distribution) changes. (But maybe I am too negative and it will all be
good.)
Thanks a lot for taking the time going through the code! Please see my
responses inline below.
> > 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;
>
> You don't need to cast here.
I was surprised, too. Without the cast I get this with gcc 4.1.2 -Wall:
mr_stresc_demo4.c:17: warning: assignment discards qualifiers from
pointer target type
> Add: // Counting how many char to escape are in instr
Ok.
> > while (*inptr != '\0') {
> > escptr = (char *)toesc;
>
> You don't need to cast here either.
See above.
> > while (*escptr != '\0') {
> > if (*inptr == *escptr++) {
>
> Add: // found it. No need to continue.
Ok.
> Also I would increment escptr separately (I don't like ++ on a same
> line as something else ;-)
Ok.
> > cnt++;
> > break;
>
> > }
> > }
> > *inptr++;
>
> No * here needed.
Sure, thanks!
> > }
> > inptr = (char *)instr;
> You don't need to cast here either.
See above.
> > retstr = (char *)malloc(strlen(inptr) + cnt + 1);
> > retptr = (char *)retstr;
> You don't need to cast here either.
See above.
> > while (*inptr != '\0') {
> > escptr = (char *)toesc;
> You don't need to cast here either.
See above.
> > while (*escptr != '\0') {
> > if (*inptr == *escptr++) {
> > *retptr++ = escchr;
>
> Rather: *retptr = ESCCHR;
Hm, why limit ourselves unnecessarily? escchr is currently an argument
to the function, i.e. the escape chararcters could be different. Don;t
you think that's a good thing?
> retptr++;
> > break;
> > }
> > }
> > *retptr++ = *inptr++;
> Idem.
Ok.
> > }
> > *retptr = '\0';
> >
> > return retstr;
> >
> > }
> >
> >
> > int main() {
> >
> > const char escchr = '\\';
> #define ESCCHR '\\'
Interesting. Probably another opportunity to improve my understanding of
things. Why is a define better than a constant? (Probably dumb but I
honestly don't know.)
> > 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;
> >
> > }
>
> Seems good to me.
Great. Please find revised new version attached (also escapes " now). If
you have an idea about the cast I'd be keen - they are still in because
of the warning mentioned above.
> But not before 2.0.9 ;-)
Sure.
> Also could we begin maybe to put these new revised and correct functions
> in new source files rather, so that we can purge the older files during
> time ?
> I'd suggest mr_string. for this function.
Right, yeah, that might be a good idea, indeed.
Do you mean mr_string.c and a corresponding mr_string.h?
> Bruno.
Cheers,
Andree
--
Andree Leidenfrost
@ Debian Developer
Sydney - Australia
#include <stdlib.h>
#include <string.h>
#include <stdio.h>
// Returns the string fed to it 'inptr' with all characters to escape given
// in 'toesc' prepended by escaping character 'escchr'.
// (Prepare strings for use in system() or popen() with this function.)
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;
// Count how many characters need escaping.
while (*inptr != '\0') {
escptr = (char *)toesc;
while (*escptr != '\0') {
if (*inptr == *escptr) {
// Found it, skip the rest.
cnt++;
break;
}
inptr++;
escptr++;
}
}
inptr = (char *)instr;
retstr = (char *)malloc(strlen(inptr) + cnt + 1);
retptr = (char *)retstr;
// Prepend specified characters with escape character.
while (*inptr != '\0') {
escptr = (char *)toesc;
while (*escptr != '\0') {
if (*inptr == *escptr) {
// Found it, skip the rest.
*retptr = escchr;
retptr++;
break;
}
escptr++;
}
*retptr = *inptr;
retptr++;
inptr++;
}
*retptr = '\0';
return retstr;
}
int main() {
#define ESCCHR '\\'
const char escape_list[4] = "`$\\\"";
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
