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

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 <[email protected]>
> Date: Sat, 18 Nov 2017 23:56:07 +0000 (UTC)
> Subject: Re: RFC: New Response type to aid rlog reponse parsing
> To: patrick keshishian <[email protected]>
> Cc: [email protected]
> 
> 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