I've added a trivial bit of code to cvs-1.10.8 that allows for the
addition of a 'u' to the format string specified in the loginfo file
and handled in logmsg.c:proc_title() .  

The advantage of this is that when running in pserver mode it becomes
possible for log messages to accurately reflect the "real" user who
made the change, rather than the user under which inetd ran the cvs
executable (it's rather upsetting to have all your log messages saying
that "nobody" made each change).  I realise that this is all far better
addressed by appropriate use of ssh, but nevertheless ..

It basically involves the following patch :

*** logmsg.c.orig       Wed Jan  5 16:35:46 2000
--- logmsg.c    Thu Sep  7 16:01:59 2000
***************
*** 555,561 ****
      void *closure;
  {
      struct logfile_info *li;
!     char *c;
  
      li = (struct logfile_info *) p->data;
      if (li->type == type)
--- 555,561 ----
      void *closure;
  {
      struct logfile_info *li;
!     char *c, *cuname = NULL;
  
      li = (struct logfile_info *) p->data;
      if (li->type == type)
***************
*** 608,613 ****
--- 608,621 ----
                                  );
                    (void) strcat (str_list, (li->rev_new
                                              ? li->rev_new : "NONE"));
+                   break;
+               case 'u':
+                 /* getcaller will not return NULL, honest */
+                   if (!cuname) cuname = getcaller();
+                   str_list = 
+                       xrealloc (str_list,
+                                 (strlen (str_list) + strlen(cuname) + 1));
+                   (void) strcat (str_list, cuname);
                    break;
                /* All other characters, we insert an empty field (but
                   we do put in the comma separating it from other

However, I'm rather concerned about the fact that the way I use
xrealloc() here is in conflict with the way it is being used elsewhere
in the same function.

As far as I can tell, I'm either missing something very important (I'd
wondered about multibyte character support, but ...), or the various
sections where xrealloc is called with "length of first string +
length of second string + 5" are being over-generous in using '5' as
the amount to be added to allow for the null terminating character.

I therefore believe (unless I've missed the aforementioned important
thing), that the following is a better patch that clarifies the code
considerably.  It appears to work sensibly in the version we tested
here (with heap-corruption-checking switched on).  I've added some
rather gratuitous comments to explain why I think each change is
correct ...


*** logmsg.c.orig       Wed Jan  5 16:35:46 2000
--- logmsg.c    Thu Sep  7 16:24:09 2000
***************
*** 555,561 ****
      void *closure;
  {
      struct logfile_info *li;
!     char *c;
  
      li = (struct logfile_info *) p->data;
      if (li->type == type)
--- 555,561 ----
      void *closure;
  {
      struct logfile_info *li;
!     char *c, *cuname = NULL;
  
      li = (struct logfile_info *) p->data;
      if (li->type == type)
***************
*** 566,578 ****
           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);
        (void) strcat (str_list, " ");
  
        if (li->type == T_TITLE)
        {
            str_list = xrealloc (str_list,
!                                strlen (str_list) + strlen (p->key) + 5);
            (void) strcat (str_list, p->key);
        }
        else
--- 566,580 ----
           You can verify that this assumption is safe by checking the
           code in add.c (add_directory) and import.c (import). */
  
!         /* +2 (space and null-terminating char) */
!       str_list = xrealloc (str_list, strlen (str_list) + 2);
        (void) strcat (str_list, " ");
  
        if (li->type == T_TITLE)
        {
+             /* +1 (null-terminating char) */
            str_list = xrealloc (str_list,
!                                strlen (str_list) + strlen (p->key) + 1);
            (void) strcat (str_list, p->key);
        }
        else
***************
*** 584,614 ****
                switch (*c)
                {
                case 's':
                    str_list =
                        xrealloc (str_list,
!                                 strlen (str_list) + strlen (p->key) + 5);
                    (void) strcat (str_list, p->key);
                    break;
                case 'V':
                    str_list =
                        xrealloc (str_list,
                                  (strlen (str_list)
!                                  + (li->rev_old ? strlen (li->rev_old) : 0)
!                                  + 10)
                                  );
                    (void) strcat (str_list, (li->rev_old
                                              ? li->rev_old : "NONE"));
                    break;
                case 'v':
                    str_list =
                        xrealloc (str_list,
                                  (strlen (str_list)
!                                  + (li->rev_new ? strlen (li->rev_new) : 0)
!                                  + 10)
                                  );
                    (void) strcat (str_list, (li->rev_new
                                              ? li->rev_new : "NONE"));
                    break;
                /* All other characters, we insert an empty field (but
                   we do put in the comma separating it from other
                   fields).  This way if future CVS versions add formatting
--- 586,628 ----
                switch (*c)
                {
                case 's':
+                   /* +1 (null-terminating char) */
                    str_list =
                        xrealloc (str_list,
!                                 strlen (str_list) + strlen (p->key) + 1);
                    (void) strcat (str_list, p->key);
                    break;
                case 'V':
+                   /* possible +4 ("NONE"), +1 (null-terminating char) */
                    str_list =
                        xrealloc (str_list,
                                  (strlen (str_list)
!                                  + (li->rev_old ? strlen (li->rev_old) : 4)
!                                  + 1)
                                  );
                    (void) strcat (str_list, (li->rev_old
                                              ? li->rev_old : "NONE"));
                    break;
                case 'v':
+                   /* possible +4 ("NONE"), +1 (null-terminating char) */
                    str_list =
                        xrealloc (str_list,
                                  (strlen (str_list)
!                                  + (li->rev_new ? strlen (li->rev_new) : 4)
!                                  + 1)
                                  );
                    (void) strcat (str_list, (li->rev_new
                                              ? li->rev_new : "NONE"));
                    break;
+               case 'u':
+                 /* getcaller will not return NULL, honest */
+                   if (!cuname) cuname = getcaller();
+                   /* +1 (null-terminating char) */
+                   str_list = 
+                       xrealloc (str_list,
+                                 (strlen (str_list) + strlen(cuname) + 1));
+                   (void) strcat (str_list, cuname);
+                   break;
                /* All other characters, we insert an empty field (but
                   we do put in the comma separating it from other
                   fields).  This way if future CVS versions add formatting
***************
*** 617,623 ****
                }
                if (*(c + 1) != '\0')
                {
!                   str_list = xrealloc (str_list, strlen (str_list) + 5);
                    (void) strcat (str_list, ",");
                }
            }
--- 631,638 ----
                }
                if (*(c + 1) != '\0')
                {
!                   /* +2 (comma and null-terminating char) */
!                   str_list = xrealloc (str_list, strlen (str_list) + 2);
                    (void) strcat (str_list, ",");
                }
            }
***************
*** 790,798 ****
  
        srepos = Short_Repository (repository);
  
        prog = xmalloc ((fmt_percent - filter) + strlen (srepos)
                        + strlen (str_list) + strlen (fmt_continue)
!                       + 10);
        (void) strncpy (prog, filter, fmt_percent - filter);
        prog[fmt_percent - filter] = '\0';
        (void) strcat (prog, "'");
--- 805,814 ----
  
        srepos = Short_Repository (repository);
  
+       /* +3 (two "'" chars and null-terminating char) */
        prog = xmalloc ((fmt_percent - filter) + strlen (srepos)
                        + strlen (str_list) + strlen (fmt_continue)
!                       + 3);
        (void) strncpy (prog, filter, fmt_percent - filter);
        prog[fmt_percent - filter] = '\0';
        (void) strcat (prog, "'");

If I'm being misled about this, then it's probably the case that my
original minimal patch should add '+10' rather than '+1' to its 
attempt to xrealloc() ...!  

-patrick.

Reply via email to