> Sorry, I didn't include his full email,
> which explain his patch quite nicely. (appended)
>
> In rethinking this, I think we should just add
> a new {u} specifier that would give us the file
> in url-encoded format, including commas.
This would help, but I think the real solution is to not let the shell parse
the arguments. I don't think there is any reason this is needed.
I don't see why URL-encoding is needed either, when it can be done without
it. This isn't to say it isn't a reasonable option to provide, I just don't
see it as the proper solution to this problem.
The patch I submitted should handle the commas case although it does take
the code in the wrong direction in terms of funk.
I didn't have any experience with the cvs source and came up with a solution
that worked for my needs. I think the _right_ answer is to change how the
script is invoked so that the shell isn't used to parse the arguments - but
it wasn't clear to me how to do that without potentially breaking other
things (or spending more time on it than I had).
> In my application, I end up converting it to
> a url form later, because I still have the
> same quoting problem - I'm outputing to a delimited
> file format, so no delimiter is going to be
> perfect. A {u} specifier would fix it for everyone
> at the source, with only the minor inconvinence
> of url-decoding the file (and directory name)
> later in their application.
>
> And they would only have to do that if they
> needed to, since they could still use the old
> {s} specifier.
>
> I will go ahead an do this, and merge it with
> your {t} specifier code. May be a couple
> of days before I get to it, tho...
What does the (new) t specifier do?
Any comments on the other two issues I brought up? The second one I think
has to be addressed.
--robs
>
> Cheers,
> -Russ
>
> ([EMAIL PROTECTED])
>
> from: http://www.mail-archive.com/bug-cvs@gnu.org/msg00975.html
> >From: Rob Saccoccio
> >Subject: cvs patches & notes
> >Date: Fri, 13 Apr 2001 10:03:52 -0700
> >
> >
> >I've identified and fixed (or worked around) some issues with cvs and
> >thought I'd pass 'em along...
> >
> >1. There's a "bogus kludge" in server.c wrapped by "#ifdef sun"
> that applies
> >to pretty old unpatched versions of SunOS. Maybe an autoconf
> script could
> >be written to test for the condition that causes the problem. I
> had a look
> >at when the OS patch was first released (and unfortunately don't
> recall the
> >details), but it was years ago. A reasonable alternative may be
> to assume
> >the kludge isn't needed and suggest that if the problem arises
> the patch be
> >applied or SUNOS_KLUDGE be defined to enable the old behavior.
> >
> >2. When operating in pserver mode, the loginfo scripts are invoked with a
> >file descriptor still open (10) which enables writing back to the client.
> >This is clearly an oversight. I came across this as I was
> trying to get the
> >server to return without waiting for the loginfo scripts to complete. In
> >turns out, the server is waiting on the closure of a FD (9) that
> is closed
> >in the helper client, but was passed to its children. I have a detailed
> >truss if you'd like it. I couldn't figure out the "right" place to close
> >the FD, so I now close it at the top of my scripts.
> >
> >3. Below is a patch against 1.11 logmsg.c that allows loginfo scripts to
> >properly handle filenames with spaces. Currently, the arguments are not
> >properly setup to handle this. The patch is backward compatible.
> >
> >--robs
>
> --- logmsg.c Fri Apr 13 12:38:43 2001
> +++ new-logmsg.c Fri Apr 13 12:38:26 2001
> @@ -552,9 +552,9 @@
> * concatenate each filename/version onto str_list
> */
> static int
> -title_proc (p, closure)
> +title_proc (p, quote)
> Node *p;
> - void *closure;
> + void *quote;
> {
> struct logfile_info *li;
> char *c;
> @@ -568,8 +568,9 @@
> You can verify that this assumption is safe by checking the
> code in add.c (add_directory) and import.c (import). */
>
> - str_list = xrealloc (str_list, strlen (str_list) + 5);
> + str_list = xrealloc (str_list, strlen (str_list) + 7);
> (void) strcat (str_list, " ");
> + (void) strcat (str_list, (char *)quote);
>
> if (li->type == T_TITLE)
> {
> @@ -624,6 +625,8 @@
> }
> }
> }
> +
> + (void) strcat (str_list, (char *)quote);
> }
> return (0);
> }
> @@ -711,9 +714,35 @@
>
> after the format string (we
>
> might skip a '}') somewhere
>
> in there... */
> + char *qp = fmt_percent - 1; /* a pointer used to find
> quotes */
> + char *quote = "";
>
> /* Grab the format string. */
>
> + /* Backward compatibility kludge. If the previous non-white space
> + character is a single quote, its there to cancel out the single
> + quotes that WERE coded around the repository and changed file
> + list. This allowed the shell to break up the args to the
> + filter program on white space, creating a seperate arg for
> + each file spec (and the repository). Since this is the
> + intended default behaviour, in order to not break existing
> + scripts we have remove it when its there and put it in
> + when its not. This parser really needs to be rewritten.
> + Another approach would have been to allow the single quote
> + to be a format character (unrecognized format chars SHOULD
> + be honored as literals and not ignored), it would break any
> + existing scripts that were expecting the comma that it is
> + currently replaced with now. */
> +
> + /* Walk back from the percent until we hit a non-white
> space char */
> + while (qp > filter && (*qp == ' ' || *qp == '\t')) --qp;
> +
> + if (*qp == '\'')
> + {
> + quote = "'";
> + *qp = ' ';
> + }
> +
> if ((*(fmt_percent + 1) == ' ') || (*(fmt_percent + 1) == '\0'))
> {
> /* The percent stands alone. This is an error. We could
> @@ -760,6 +789,15 @@
> fmt_continue = fmt_end;
> }
>
> + /* Walk forward from fmt_continue until we hit a non-white space
> char */
> + qp = fmt_continue;
> + while (*qp != '\0' && (*qp == ' ' || *qp == '\t')) ++qp;
> +
> + if (*qp == '\'' && *quote != '\0')
> + {
> + *qp = ' ';
> + }
> +
> len = fmt_end - fmt_begin;
> str_list_format = xmalloc (len + 1);
> strncpy (str_list_format, fmt_begin, len);
> @@ -777,13 +815,13 @@
> if (str_list_format[0] != '\0')
> {
> type = T_TITLE;
> - (void) walklist (changes, title_proc, NULL);
> + (void) walklist (changes, title_proc, quote);
> type = T_ADDED;
> - (void) walklist (changes, title_proc, NULL);
> + (void) walklist (changes, title_proc, quote);
> type = T_MODIFIED;
> - (void) walklist (changes, title_proc, NULL);
> + (void) walklist (changes, title_proc, quote);
> type = T_REMOVED;
> - (void) walklist (changes, title_proc, NULL);
> + (void) walklist (changes, title_proc, quote);
> }
>
> free (str_list_format);
> @@ -794,13 +832,15 @@
>
> prog = xmalloc ((fmt_percent - filter) + strlen (srepos)
> + strlen (str_list) +
> strlen (fmt_continue)
> - + 10);
> + + 12);
> (void) strncpy (prog, filter, fmt_percent - filter);
> prog[fmt_percent - filter] = '\0';
> - (void) strcat (prog, "'");
> + if (*quote == '\0') (void) strcat (prog, "'");
> + (void) strcat (prog, quote);
> (void) strcat (prog, srepos);
> + (void) strcat (prog, quote);
> (void) strcat (prog, str_list);
> - (void) strcat (prog, "'");
> + if (*quote == '\0') (void) strcat (prog, "'");
> (void) strcat (prog, fmt_continue);
>
> /* To be nice, free up some memory. */
>
> -----
>
>
>
_______________________________________________
Bug-cvs mailing list
[EMAIL PROTECTED]
http://mail.gnu.org/mailman/listinfo/bug-cvs