From: Date: April 20 2007 12:58am
Subject: Review 1 of patch 2 for #27894: mysqlbinlog formats timestamp wrong in comment
List-Archive: http://lists.mysql.com/internals/34561
Message-Id: <99AAACF8-C4E7-4920-A249-18CC8808CAFE@mysql.com>
MIME-Version: 1.0 (Apple Message framework v752.3)
Content-Type: multipart/signed; protocol="application/pgp-signature"; micalg=pgp-sha1; boundary="Apple-Mail-18--999680631"
Content-Transfer-Encoding: 7bit

--Apple-Mail-18--999680631
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain; charset=ISO-8859-1; delsp=yes; format=flowed


On 19 Apr 2007, at 16:51, Jorge Bernal Ordov=E1s wrote:

> OK, here's the second version. It seems Apple Mail sends .patch =20
> files as binary attachments, so I guess that's the reason for being =20=

> 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 =20
>> think of a better way to do that.
>>
> I've basically set a fixed size for buffz. It could be another way =20
> to do this (in fact, there are lots of ways), but I don't see how =20
> to do that without turning the code in something too comples.


Hi Koke.

> --- mysql-5.1-trunk/mysys/mf_iocache2.c	2007-02-24 =20
> 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 =20=

> the
> @@ -337,8 +338,10 @@
>      minimum_width=3D 0;
>      precision=3D 0;
>      minimum_width_sign=3D 1;
> +    is_zero_padded=3D FALSE;
>      /* Skip if max size is used (to be compatible with printf) */
>      while (*fmt =3D=3D '-') { fmt++; minimum_width_sign=3D -1; }
> +    while (*fmt =3D=3D '0') { fmt++; is_zero_padded=3D TRUE; }

Yes!

>      if (*fmt =3D=3D '*') {
>        precision=3D (int) va_arg(args, int);
>        fmt++;
> @@ -390,6 +393,15 @@
> 	length2=3D (uint) (int10_to_str((long) iarg,buff, -10) - buff);
>        else
> 	length2=3D (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 =20
surrounding code.

But, in this case, it's not the same "17" -- the others are locations =20=

to represent numbers, whereas this is padding.  "%09000d" is at least =20=

legal, though unlikely, right?  That would try to write 8983 bytes =20
past the end of buffz.

I suggest making a for-loop and write chunks of padding or (I don't =20
like as much) use my_alloca() to allocate one up front, and pack it, =20
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+=3D 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: +1 408 213 6740                         sip:6740@stripped



--Apple-Mail-18--999680631
content-type: application/pgp-signature; x-mac-type=70674453;
	name=PGP.sig
content-description: This is a digitally signed message part
content-disposition: inline; filename=PGP.sig
content-transfer-encoding: 7bit

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (Darwin)

iD8DBQFGJ/QL/peCpMTxrLsRAhG6AJsHhJd5Ulub636iUqneRKhP2pQgqgCfaqOS
CK6W47Yk7kg64XcqeeAtkY8=
=0dke
-----END PGP SIGNATURE-----

--Apple-Mail-18--999680631--