From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp50.i.mail.ru (smtp50.i.mail.ru [94.100.177.110]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 63532469710 for ; Sun, 7 Jun 2020 15:38:48 +0300 (MSK) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 13.0 \(3608.60.0.2.5\)) From: Chris Sosnin In-Reply-To: <380bf818-5886-f6ef-67a6-7416bc16eb3b@tarantool.org> Date: Sun, 7 Jun 2020 15:38:46 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: <2C9404FB-D44A-4772-A83D-C7A9F36AA85D@tarantool.org> References: <401a596ba9864ddf4ffc687f4313a60dbc664a28.1591371404.git.k.sosnin@tarantool.org> <380bf818-5886-f6ef-67a6-7416bc16eb3b@tarantool.org> Subject: Re: [Tarantool-patches] [PATCH 1/2] Allow leading and trailing whitespaces in FromString List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Serge Petrenko Cc: tarantool-patches@dev.tarantool.org, Vladislav Shpilevoy Hi! Thank you for the review! > On 6 Jun 2020, at 16:48, Serge Petrenko = wrote: >=20 >=20 > 05.06.2020 18:41, Chris Sosnin =D0=BF=D0=B8=D1=88=D0=B5=D1=82: >> This is needed for SQL, since these whitespaces are allowed in >> conversions from string. >>=20 >> Needed for tarantool/tarantool#4415 >=20 > Hi! Thanks for the patch! >=20 > See one comment below. >=20 >> --- >> decNumber.c | 44 +++++++++++++++++++++++++++++--------------- >> 1 file changed, 29 insertions(+), 15 deletions(-) >>=20 >> diff --git a/decNumber.c b/decNumber.c >> index 85deb13..7c53bc3 100644 >> --- a/decNumber.c >> +++ b/decNumber.c >> @@ -555,9 +555,9 @@ decNumber * decNumberFromString(decNumber *dn, = const char chars[], >> Unit *allocres=3DNULL; // -> allocated result, iff = allocated >> Int d=3D0; // count of digits found in = decimal part >> const char *dotchar=3DNULL; // where dot was found >> - const char *cfirst=3Dchars; // -> first character of = decimal part >> + const char *cfirst=3DNULL; // -> first character of = decimal part >> const char *last=3DNULL; // -> last digit of decimal = part >> - const char *c; // work >> + const char *c, *p; // work >> Unit *up; // .. >> #if DECDPUN>1 >> Int cut, out; // .. >> @@ -571,6 +571,9 @@ decNumber * decNumberFromString(decNumber *dn, = const char chars[], >> #endif >> do { // status & malloc protection >> + for (; *chars=3D=3D' '; ++chars); // skip leading whitespaces >> + cfirst=3Dchars; >> + >> for (c=3Dchars;; c++) { // -> input character >> if (*c>=3D'0' && *c<=3D'9') { // test for Arabic digit >> last=3Dc; >> @@ -590,13 +593,14 @@ decNumber * decNumberFromString(decNumber *dn, = const char chars[], >> cfirst++; >> continue;} >> } >> + for (p=3Dc; *p=3D=3D' '; ++p); > Looks like this `for` loop should go below. >> // *c is not a digit, or a valid +, -, or '.' >> break; >> } // c >> =20 >=20 > Here, ^ (This guy's formatting is confusing as hell, I know) You are right, thank you for the catch! I applied the change on the branch. >=20 >> if (last=3D=3DNULL) { // no digits yet >> status=3DDEC_Conversion_syntax;// assume the worst >> - if (*c=3D=3D'\0') break; // and no more to come... >> + if (*p=3D=3D'\0') break; // and no more to come... >> #if DECSUBSET >> // if subset then infinities and NaNs are not allowed >> if (!set->extended) break; // hopeless >> @@ -626,7 +630,8 @@ decNumber * decNumberFromString(decNumber *dn, = const char chars[], >> // now either nothing, or nnnn payload, expected >> // -> start of integer and skip leading 0s [including plain 0] >> for (cfirst=3Dc; *cfirst=3D=3D'0';) cfirst++; >> - if (*cfirst=3D=3D'\0') { // "NaN" or "sNaN", maybe = with all 0s >> + for (p=3Dcfirst; *p=3D=3D' '; ++p); >> + if (*p=3D=3D'\0') { // "NaN" or "sNaN", maybe = with all 0s >> status=3D0; // it's good >> break; // .. >> } >> @@ -635,7 +640,8 @@ decNumber * decNumberFromString(decNumber *dn, = const char chars[], >> if (*c<'0' || *c>'9') break; // test for Arabic digit >> last=3Dc; >> } >> - if (*c!=3D'\0') break; // not all digits >> + for (p=3Dc; *p=3D=3D' '; ++p); >> + if (*p!=3D'\0') break; // not all digits >> if (d>set->digits-1) { >> // [NB: payload in a decNumber can be full length unless >> // clamped, in which case can only be digits-1] >> @@ -647,7 +653,7 @@ decNumber * decNumberFromString(decNumber *dn, = const char chars[], >> bits=3Ddn->bits; // for copy-back >> } // last=3D=3DNULL >> - else if (*c!=3D'\0') { // more to process... >> + else if (p=3D=3Dc && *p!=3D'\0') { // more to process... >> // had some digits; exponent is only valid sequence now >> Flag nege; // 1=3Dnegative exponent >> const char *firstexp; // -> first significant exponent = digit >> @@ -659,16 +665,18 @@ decNumber * decNumberFromString(decNumber *dn, = const char chars[], >> c++; // to (possible) sign >> if (*c=3D=3D'-') {nege=3D1; c++;} >> else if (*c=3D=3D'+') c++; >> - if (*c=3D=3D'\0') break; >> - >> - for (; *c=3D=3D'0' && *(c+1)!=3D'\0';) c++; // strip = insignificant zeros >> + for (p=3Dc; *p=3D=3D' '; ++p); >> + if (*p=3D=3D'\0') break; >> + // strip insignificant zeros >> + for (; *c=3D=3D'0' && *(c+1)!=3D'\0' && *(c+1)!=3D' ';) c++; >> firstexp=3Dc; // save exponent = digit place >> for (; ;c++) { >> if (*c<'0' || *c>'9') break; // not a digit >> exponent=3DX10(exponent)+(Int)*c-(Int)'0'; >> } // c >> // if not now on a '\0', *c must not be a digit >> - if (*c!=3D'\0') break; >> + for (p=3Dc; *p=3D=3D' '; ++p); >> + if (*p!=3D'\0') break; >> // (this next test must be after the syntax checks) >> // if it was too long the exponent may have wrapped, so check >> @@ -681,6 +689,11 @@ decNumber * decNumberFromString(decNumber *dn, = const char chars[], >> status=3D0; // is OK >> } // stuff after digits >> + else if (*p!=3D'\0') { // whitespaces in the middle >> + status=3DDEC_Conversion_syntax; >> + break; >> + } >> + >> // Here when whole string has been inspected; syntax is good >> // cfirst->first digit (never dot), last->last digit (ditto) >> @@ -7739,12 +7752,13 @@ static decNumber *decDecap(decNumber *dn, = Int drop) { >> /* if (decBiStr(test, "mike", "MIKE")) ... = */ >> /* = ------------------------------------------------------------------ */ >> static Flag decBiStr(const char *targ, const char *str1, const char = *str2) { >> - for (;;targ++, str1++, str2++) { >> + const char *end=3Dstrchr(targ, ' '); >> + if (end=3D=3DNULL) >> + end=3Dtarg+strlen(targ); >> + for (;targ> if (*targ!=3D*str1 && *targ!=3D*str2) return 0; >> - // *targ has a match in one (or both, if terminator) >> - if (*targ=3D=3D'\0') break; >> - } // forever >> - return 1; >> + } // for >> + return *str1=3D=3D'\0'; >> } // decBiStr >> /* = ------------------------------------------------------------------ */ >=20 > --=20 > Serge Petrenko