At Fri, 10 Sep 2021 04:33:53 +0000, "kuroda.hay...@fujitsu.com"
<kuroda.hay...@fujitsu.com> wrote in
> Dear Hou,
>
> > I found one minor thing in the patch.
>
> Thanks!
>
> > Is it better to use Abs(padding) here ?
> > Although some existing codes are using " padding > 0 ? padding : -padding ".
>
> +1, fixed.
>
> And I found that some comments were not added above padding calculation,
> so added.
Thanks for the new version. I don't object for reusing
process_log_prefix_padding, but I still find it strange that the
function with the name 'process_padding' is in string.c. If we move
it to string.c, I'd like to name it "pg_fast_strtoi" or something and
change the interface to int pg_fast_strtoi(const char *p, char
**endptr) that is (roughly) compatible to strtol. What do (both) you
think about this?
+ /*
+ * Search application_name and replace it if found.
+ *
+ * We search paramters from the end because the later
+ * one have higher priority. See also the above comment.
+ */
+ for (i = n - 1; i >= 0; i--)
+ {
+ if (strcmp(keywords[i], "application_name") == 0)
+ {
+ parse_pgfdw_appname(&buf, values[i]);
+ values[i] = buf.data;
+
+ /*
+ * If the input format is wrong and the string
becomes '\0',
+ * this parameter is no longer used.
+ * We conitnue searching application_name.
+ */
+ if (*values[i] != '\0')
+ break;
+ }
+ }
I didn't fully checked in what case parse_pgfdw_appname gives "" as
result, I feel that we should use the original value in that
case. That is,
> parse_pgfdw_appname(&buf, vaues[i]);
>
> /* use the result if any, otherwise use the original string */
> if (buf.data[0] != 0)
> values[i] = buf.data;
>
> /* break if it results in non-empty string */
> if (values[0][0] != 0)
> break;
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
