Re: RFC: New Response type to aid rlog reponse parsing

[patrick keshishian]

Hi Thorsten,

Sorry for the delayed reply (project I'm working on is taking
far too many cycles).

On Sun, Nov 19, 2017 at 07:58:47PM -0800, patrick keshishian wrote:
> ---------- Forwarded message ----------
> From: Thorsten Glaser <address@hidden>
> Date: Sat, 18 Nov 2017 23:56:07 +0000 (UTC)
> Subject: Re: RFC: New Response type to aid rlog reponse parsing
> To: patrick keshishian <address@hidden>
> Cc: address@hidden
> 
> patrick keshishian dixit:
> 
> >Attached is a patch implementing LOGM change against cvs-1.11.23
> 
> I've implemented it differently, especially in order to never
> send partial lines to the client (in a network packet). If you
> wish, have a look at / review the following patch.

Just from reading the diff it looks OK. However, I didn't comprehend
the sending "partial lines to the client" bit.


> I'm currently sanity.sh-testing it, this will take the night.

Hopefully your tests don't show any regression.

I can patch my CVS server with your diff when I break away from
my work.

Thanks for the update!

Happy Thanksgiving!

--patrick

p.s., sending from a different account (not subscribed to CVS list)
hopefully it'll make it.


> --- src/gnu/usr.bin/cvs/src/buffer.c:1.2      Sat Oct 22 14:33:29 2016
> +++ src/gnu/usr.bin/cvs/src/buffer.c  Sat Nov 18 22:05:10 2017
> @@ -1052,7 +1055,14 @@ buf_copy_lines (struct buffer *outbuf, s
>       }
> 
>       /* Put in the command.  */
> -     buf_append_char (outbuf, command);
> +     switch (command) {
> +     case CVS_OUTPUT_EX_LOGM:
> +             buf_output0 (outbuf, "LOGM");
> +             break;
> +     default:
> +             buf_append_char (outbuf, command);
> +             break;
> +     }
>       buf_append_char (outbuf, ' ');
> 
>       if (inbuf->data != nldata)
> --- src/gnu/usr.bin/cvs/src/client.c:1.8      Sat Aug 12 01:51:22 2017
> +++ src/gnu/usr.bin/cvs/src/client.c  Sat Nov 18 22:05:11 2017
> @@ -3079,6 +3079,7 @@ struct response responses[] =
>         rs_optional),
>      RSP_LINE("M", handle_m, response_type_normal, rs_essential),
>      RSP_LINE("Mbinary", handle_mbinary, response_type_normal, rs_optional),
> +    RSP_LINE("LOGM", handle_m, response_type_normal, rs_optional),
>      RSP_LINE("E", handle_e, response_type_normal, rs_essential),
>      RSP_LINE("F", handle_f, response_type_normal, rs_optional),
>      RSP_LINE("MT", handle_mt, response_type_normal, rs_optional),
> --- src/gnu/usr.bin/cvs/src/cvs.h:1.7 Fri Oct 21 16:41:16 2016
> +++ src/gnu/usr.bin/cvs/src/cvs.h     Sat Nov 18 22:05:12 2017
> @@ -920,11 +921,15 @@ void tag_check_valid (const char *, int,
> 
>  /* From server.c and documented there.  */
>  void cvs_output (const char *, size_t);
> +void cvs_output_ex (const char *, size_t, int);
>  void cvs_output_binary (char *, size_t);
>  void cvs_outerr (const char *, size_t);
>  void cvs_flusherr (void);
>  void cvs_flushout (void);
>  void cvs_output_tagged (const char *, const char *);
> +int supported_response (const char *);
> +
> +#define CVS_OUTPUT_EX_LOGM   0x80000001
> 
>  extern const char *global_session_id;
> 
> --- src/gnu/usr.bin/cvs/src/log.c:1.1.101.2   Tue Apr 19 20:33:18 2005
> +++ src/gnu/usr.bin/cvs/src/log.c     Sat Nov 18 22:05:12 2017
> @@ -145,6 +148,7 @@ static void log_version (struct log_data
>                               RCSNode *, RCSVers *, int);
>  static int log_branch (Node *, void *);
>  static int version_compare (const char *, const char *, int);
> +static void logm_output (const char *);
> 
>  static struct log_data log_data;
>  static int is_rlog;
> @@ -1681,11 +1685,10 @@ log_version (struct log_data *log_data,
>       cvs_output ("*** empty log message ***\n", 0);
>      else
>      {
> +     /* assert: last thing cvs_output???ed was a newline */
>       /* FIXME: Technically, the log message could contain a null
>             byte.  */
> -     cvs_output (p->data, 0);
> -     if (((char *)p->data)[strlen (p->data) - 1] != '\n')
> -         cvs_output ("\n", 1);
> +     logm_output(p->data);
>      }
>  }
> 
> @@ -1780,3 +1783,23 @@ version_compare (const char *v1, const c
>           ++v2;
>      }
>  }
> +
> +static void
> +logm_output(const char *str)
> +{
> +     /* assert: str is not empty */
> +     size_t len = strlen(str);
> +     int buftag = 'M';
> +     static char has_logm = 0;
> +
> +     if (server_active) {
> +             if (!has_logm)
> +                     has_logm = supported_response("LOGM") ? 1 : 2;
> +             if (has_logm == 1)
> +                     buftag = CVS_OUTPUT_EX_LOGM;
> +     }
> +
> +     cvs_output_ex(str, len, buftag);
> +     if (/*len > 0 &&*/ str[len - 1] != '\n')
> +             cvs_output_ex("\n", 1, buftag);
> +}
> --- src/gnu/usr.bin/cvs/src/server.c:1.14     Sat Aug 12 01:08:25 2017
> +++ src/gnu/usr.bin/cvs/src/server.c  Sat Nov 18 22:05:12 2017
> @@ -548,8 +553,8 @@ alloc_pending_warning (size_t size)
> 
> 
> 
> -static int
> -supported_response (char *name)
> +int
> +supported_response (const char *name)
>  {
>      struct response *rs;
> 
> @@ -7703,6 +7708,12 @@ krb_encrypt_buffer_initialize( struct bu
>  void
>  cvs_output (const char *str, size_t len)
>  {
> +    cvs_output_ex (str, len, 'M');
> +}
> +
> +void
> +cvs_output_ex (const char *str, size_t len, int buftag)
> +{
>      if (len == 0)
>       len = strlen (str);
>  #ifdef SERVER_SUPPORT
> @@ -7711,7 +7722,7 @@ cvs_output (const char *str, size_t len)
>       if (buf_to_net)
>       {
>           buf_output (saved_output, str, len);
> -         buf_copy_lines (buf_to_net, saved_output, 'M');
> +         buf_copy_lines (buf_to_net, saved_output, buftag);
>       }
>  # if HAVE_SYSLOG_H
>       else
> @@ -7726,7 +7737,7 @@ cvs_output (const char *str, size_t len)
>       if (protocol)
>       {
>           buf_output (saved_output, str, len);
> -         buf_copy_lines (protocol, saved_output, 'M');
> +         buf_copy_lines (protocol, saved_output, buftag);
>           buf_send_counted (protocol);
>       }
>  # if HAVE_SYSLOG_H
> 
> 
> bye,
> //mirabilos