Review 1 of patch 2 for #27894: mysqlbinlog formats timestamp wrong in comment

[Chad MILLER]

On 19 Apr 2007, at 16:51, Jorge Bernal Ordovás wrote:

> OK, here's the second version. It seems Apple Mail sends .patch  
> files as binary attachments, so I guess that's the reason for being  
> filtered.
>
> Just in case it don't make it to the list:
> http://people.warp.es/~koke/patches/27894-fix-printf_2.diff
> [...]
>>
>> In light of the constancy of the "buffz" size, perhaps you can  
>> think of a better way to do that.
>>
> I've basically set a fixed size for buffz. It could be another way  
> to do this (in fact, there are lots of ways), but I don't see how  
> to do that without turning the code in something too comples.


Hi Koke.

> --- mysql-5.1-trunk/mysys/mf_iocache2.c	2007-02-24  
> 02:43:10.000000000 -0800
> +++ mysql-5.1/mysys/mf_iocache2.c	2007-04-19 13:32:13.000000000 -0700
> @@ -299,6 +299,7 @@
>    uint minimum_width; /* as yet unimplemented */
>    uint minimum_width_sign;
>    uint precision; /* as yet unimplemented for anything but %b */
> +  my_bool is_zero_padded;
>    /*
>      Store the location of the beginning of a format directive, for  
> the
> @@ -337,8 +338,10 @@
>      minimum_width= 0;
>      precision= 0;
>      minimum_width_sign= 1;
> +    is_zero_padded= FALSE;
>      /* Skip if max size is used (to be compatible with printf) */
>      while (*fmt == '-') { fmt++; minimum_width_sign= -1; }
> +    while (*fmt == '0') { fmt++; is_zero_padded= TRUE; }

Yes!

>      if (*fmt == '*') {
>        precision= (int) va_arg(args, int);
>        fmt++;
> @@ -390,6 +393,15 @@
> 	length2= (uint) (int10_to_str((long) iarg,buff, -10) - buff);
>        else
> 	length2= (uint) (int10_to_str((long) (uint) iarg,buff,10)- buff);
> +	if (minimum_width > length2)
> +	{
> +		char buffz[17];

Tabs again.  :\  ':set expandtabs" if you're using vi.

I hate to see the magical value "17".  I'm ashamed to see it in the  
surrounding code.

But, in this case, it's not the same "17" -- the others are locations  
to represent numbers, whereas this is padding.  "%09000d" is at least  
legal, though unlikely, right?  That would try to write 8983 bytes  
past the end of buffz.

I suggest making a for-loop and write chunks of padding or (I don't  
like as much) use my_alloca() to allocate one up front, and pack it,  
and then write it.

> +		if (is_zero_padded)
> +			memset(buffz, '0', minimum_width - length2);
> +		else
> +			memset(buffz, ' ', minimum_width - length2);
> +		my_b_write(info, buffz, minimum_width - length2);
> +	}
>        out_length+= length2;
>        if (my_b_write(info, buff, length2))
> 	goto err;

It's getting very close.

- chad

--
Chad Miller, Software Developer                         chad@stripped
MySQL Inc., www.mysql.com
Orlando, Florida, USA                                13-20z,  UTC-0400
Office: [phone]                         sip:6740@stripped



Attachment: [application/pgp-signature] This is a digitally signed message part PGP.sig