Re: [PATCH 8/6 v2] receive-pack: Send internal errors over side-band #2

["Shawn O. Pearce" <spearce@spearce.org>]

Johannes Sixt <j.sixt@viscovery.net> wrote:
> Shawn O. Pearce schrieb:
>> +static void report_message(const char *prefix, const char *err, va_list params)
>> +{
>> +	int sz = strlen(prefix);
>> +	char msg[4096];
>> +
>> +	strncpy(msg, prefix, sz);
>> +	sz += vsnprintf(msg + sz, sizeof(msg) - sz, err, params);
>> +	if (sz > (sizeof(msg) - 1))
>> +		sz = sizeof(msg) - 1;
>> +	msg[sz++] = '\n';
>
> Sorry, still no joy - the terminating NUL is missing (I should have  
> noticed this in your v1 already).

Why is it necessary?

Once the msg buffer is prepared, its written using its length, sz,
not its NUL termination status.  Neither send_sideband() nor xwrite()
care about NUL termination.

The only reason to put a NUL onto this buffer is so you can do
"p msg" within GDB and get a useful result.  We don't typically do
this in these contexts.

Or did my MTA inject additional C code I didn't write?

> I suggest to forgo the length check for 
> simplicity because this function is only called with data that is already 
> guaranteed to be less than 1000 bytes, i.e.:
>
> 	strncpy(msg, prefix, sz);
> 	/* data is guaranteed to fit due to packet length limit in  
> read_head_info() */

Yea, that's true now.  We probably could have used 1200 bytes for
the msg buffer, rather than 4096.  Even the huge warnings about
current branch behavior are broken up into one string per line.

-- 
Shawn.