At Thu, 23 Dec 2021 23:10:38 +0900, Fujii Masao <masao.fujii@oss.nttdata.com> wrote in
>
>
> On 2021/12/17 16:50, Kyotaro Horiguchi wrote:
> > Thus rewriting the code we're focusing on like the following would
> > make sense to me.
> >
> >> if (strcmp(keywords[i], "application_name") == 0)
> >> {
> >> values[i] = process_pgfdw_appname(values[i]);
> >>
> >> /*
> >> * Break if we have a non-empty string. If we end up failing with
> >> * all candidates, fallback_application_name would work.
> >> */
> >> if (appanme[0] != '\0')
(appname?)
> >> break;
> >> }
>
> I'm ok to remove the check "values[i] != NULL", but think that it's
> better to keep the other check "*(values[i]) != '\0'" as it
> is. Because *(values[i]) can be null character and it's a waste of
> cycles to call process_pgfdw_appname() in that case.
Right. I removed too much.
> > Thanks for revisiting.
> >
> >> #1. use "[unknown]"
> >> #2. add the check but not use "[unknown]"
> >> #3. don't add the check (i.e., what the current patch does)
> >>
> >> For now, I'm ok to choose #2 or #3.
> > As I said before, given that we don't show "unkown" or somethig like
> > as the fallback, I'm fine with not having a NULL check since anyway it
> > bumps into SEGV immediately. In short I'm fine with #3 here.
>
> Yep, let's use #3 approach.
Agreed.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center