<div dir="ltr"><div><br><div class="gmail_quote"><div dir="ltr">ср, 18 июл. 2018 г. в 20:10, Alexander Turenko <<a href="mailto:alexander.turenko@tarantool.org">alexander.turenko@tarantool.org</a>>:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Two minor comments are below.<br>
<br>
WBR, Alexander Turenko.<br>
<br>
On Wed, Jul 18, 2018 at 06:57:39PM +0300, Nikita Tatunov wrote:<br>
> ср, 18 июл. 2018 г. в 18:53, Alex Khatskevich<br>
> <[1]<a href="mailto:avkhatskevich@tarantool.org" target="_blank">avkhatskevich@tarantool.org</a>>:<br>
> <br>
> Once you have fixed comments, please apply a new full diff at the end<br>
> of email, to<br>
> continue review process.<br>
> Answer with a full diff to this email please (just paste as a plain<br>
> text output of `git diff` command)<br>
> On 18.07.2018 18:24, Nikita Tatunov wrote:<br>
> <br>
> -/*<br>
> - * For LIKE and GLOB matching on EBCDIC machines, assume that every<br>
> - * character is exactly one byte in size. Also, provde the Utf8Read()<br>
> - * macro for fast reading of the next character in the common case<br>
> where<br>
> - * the next character is ASCII.<br>
> +/**<br>
> + * Providing there are symbols in string s this macro returns<br>
> + * UTF-8 code of character and pushes pointer to the next symbol<br>
> + * in the string. Otherwise return code is SQL_END_OF_STRING.<br>
<br>
Nitpicking: pushes -> promotes?<br>
<br></blockquote><div><br></div><div>Both should be fine, I guess. But ok, changed it.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> */<br>
> -#define Utf8Read(s, e) ucnv_getNextUChar(pUtf8conv, &s, e, &status)<br>
> +#define Utf8Read(s, e) (s < e ?\<br>
> + ucnv_getNextUChar(pUtf8conv, &s, e, &status) : 0)<br>
> +<br>
> +#define SQL_END_OF_STRING 0<br>
<br>
Always wrap argument usages of a function-like macro within its body<br>
with parenthesis (consider example I give in the previous email). It is<br>
usual way to handle the fact that a macros works as text replacement,<br>
so, say<br>
<br>
#define SUB(x, y) x - y<br>
<br>
will give the wrong result for the expression SUB(10, 5 - 3), while<br>
<br>
#define SUB(x, y) ((x) - (y))<br>
<br>
will produce the correct result.<br></blockquote><div><br></div><div>Fixed.</div><div><br></div><div><span style="font-size:small;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">Also made few minor codestyle fixes in tests compared to prev. patch.</span><br class="gmail-Apple-interchange-newline"><br><br> diff --git a/src/box/sql/func.c b/src/box/sql/func.c</div><div>index c06e3bd..a9784cc 100644</div><div>--- a/src/box/sql/func.c</div><div>+++ b/src/box/sql/func.c</div><div>@@ -617,13 +617,16 @@ struct compareInfo {</div><div> <span style="white-space:pre"> </span>u8 noCase;<span style="white-space:pre"> </span>/* true to ignore case differences */</div><div> };</div><div> </div><div>-/*</div><div>- * For LIKE and GLOB matching on EBCDIC machines, assume that every</div><div>- * character is exactly one byte in size. Also, provde the Utf8Read()</div><div>- * macro for fast reading of the next character in the common case where</div><div>- * the next character is ASCII.</div><div>+/**</div><div>+ * Providing there are symbols in string s this</div><div>+ * macro returns UTF-8 code of character and</div><div>+ * promotes pointer to the next symbol in the string. </div><div>+ * Otherwise return code is SQL_END_OF_STRING.</div><div> */</div><div>-#define Utf8Read(s, e) ucnv_getNextUChar(pUtf8conv, &s, e, &status)</div><div>+#define Utf8Read(s, e) (((s) < (e)) ?\</div><div>+<span style="white-space:pre"> </span>ucnv_getNextUChar(pUtf8conv, &s, e, &status) : 0)</div><div>+</div><div>+#define SQL_END_OF_STRING 0</div><div> <br></div></div></div></div>