Re: Allow escape in application_name

From: Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com>
To: Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>
Cc: kuroda(dot)hayato(at)fujitsu(dot)com, houzj(dot)fnst(at)fujitsu(dot)com, ikedamsh(at)oss(dot)nttdata(dot)com, pgsql-hackers(at)lists(dot)postgresql(dot)org, tgl(at)sss(dot)pgh(dot)pa(dot)us
Subject: Re: Allow escape in application_name
Date: 2021-12-23 14:10:38
Message-ID: 91374436-637b-1aad-0f60-a1fc8b9f92e2@oss.nttdata.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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')
>> 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.

> 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.

Regards,

--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Fujii Masao 2021-12-23 14:42:05 Re: sequences vs. synchronous replication
Previous Message Andrew Dunstan 2021-12-23 13:50:28 Re: Buildfarm support for older versions