Re: cvs patches & notes

[Russ Tremain]

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. */

-----