> -----Original Message----- > From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Petr Jelinek > Sent: Thursday, March 10, 2016 11:01 AM > To: Kaigai Kouhei(海外 浩平); pgsql-hackers@postgresql.org > Subject: Re: [HACKERS] Reworks of CustomScan serialization/deserialization > > On 10/03/16 02:18, Kouhei Kaigai wrote: > > > >> I am not sure I like the fact that we have this EXTNODENAME_MAX_LEN and > >> now the CUSTOM_NAME_MAX_LEN with the same length and also they are both > >> same lenght as NAMEDATALEN I wonder if this shouldn't be somehow > >> squished to less defines. > >> > > Hmm. I just followed the manner in extensible.c, because this label was > > initially NAMEDATALEN, then Robert changed it with EXTNODENAME_MAX_LEN. > > I guess he avoid to apply same label on different entities - NAMEDATALEN > > is a limitation for NameData type, but identifier of extensible-node and > > custom-scan node are not restricted by. > > > > Makes sense. > > >> Also in RegisterCustomScanMethods > >> + Assert(strlen(methods->CustomName) <= CUSTOM_NAME_MAX_LEN); > >> > >> Shouldn't this be actually "if" with ereport() considering this is > >> public API and extensions can pass anything there? (for that matter same > >> is true for RegisterExtensibleNodeMethods but that's already committed). > >> > > Hmm. I don't have clear answer which is better. The reason why I put > > Assert() here is that only c-binary extension uses this interface, thus, > > author will fix up the problem of too long name prior to its release. > > Of course, if-with-ereport() also informs extension author the name is > > too long. > > One downside of Assert() may be, it makes oversight if --enable-cassert > > was not specified. > > > > Well that's exactly my problem, this should IMHO throw error even > without --enable-cassert. It's not like it's some performance sensitive > API where if would be big problem, ensuring correctness of the input is > more imporant here IMHO. > We may need to fix up RegisterExtensibleNodeMethods() first.
Also, length limitation is (EXTNODENAME_MAX_LEN-1) because the last byte is consumed by '\0' character. In fact, hash, match and keycopy function of HTAB for string keys deal with the first (keysize - 1) bytes. So, strkey(extnodename) == EXTNODENAME_MAX_LEN is not legal. Thanks, -- NEC Business Creation Division / PG-Strom Project KaiGai Kohei <kai...@ak.jp.nec.com>
pgsql-v9.6-extensible-namelen-check-by-ereport.patch
Description: pgsql-v9.6-extensible-namelen-check-by-ereport.patch
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers