[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: cvs patches & notes
From: |
Russ Tremain |
Subject: |
Re: cvs patches & notes |
Date: |
Thu, 10 May 2001 19:59:16 -0700 |
At 3:45 PM -0700 5/10/01, Greg Klanderman wrote:
>>>>>> Russ Tremain <russt@ebay.sun.com> writes:
>
>> Hi Rob -
>> In regards to your logmsg.c patch, I have been able
>> to kludge around the white space problem by use
>> %{vsV} in the format specifier, and then joining
>> the arguments passed by the shell back together
>> in my loginfo script. I can then recognize
>> the white space because it is trapped between
>> the two revision strings. Ugly hack though.
Greg -
>
>nice.. of course I could put commas in my file name
>like this: "foo,2.3 1.3,bar"!
Yep, it will break my code if you have a filename:
"foo,2.3 1.3,bar"
So far, no one has been this perverted... :)
Sounds like you are familiar with this problem.
>> Another thought would be to modifiy logmsg.c
>> to use url-style encoding of the file name.
>
>> This would solve the problem once and for all.
>> Could make it a config option in order to make it backward
>> compatible.
>
>> What are your thoughts? I'm cc'ing Greg Klanderman
>> on this because he has also done some recent work in this
>> area.
>
>what was Rob's solution?
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.
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...
Cheers,
-Russ
(russ.tremain@sun.com)
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. */
-----