From: | Daniel Gustafsson <daniel(at)yesql(dot)se> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | Postgres hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: Lift line-length limit for pg_service.conf |
Date: | 2020-09-22 11:50:11 |
Message-ID: | 1318CD66-C97D-4BA1-87D1-6CC2B1DC78F2@yesql.se |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
> On 21 Sep 2020, at 17:09, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> Daniel Gustafsson <daniel(at)yesql(dot)se> writes:
>> The pg_service.conf parsing thread [0] made me realize that we have a hardwired
>> line length of max 256 bytes. Lifting this would be in line with recent work
>> for ecpg, pg_regress and pg_hba (784b1ba1a2 and 8f8154a50). The attached moves
>> pg_service.conf to use the new pg_get_line_append API and a StringInfo to lift
>> the restriction. Any reason not to do that while we're lifting other such limits?
>
> +1. I'd been thinking of looking around at our fgets calls to see
> which ones need this sort of work, but didn't get to it yet.
I took a quick look and found the TOC parsing in pg_restore which used a 100
byte buffer and then did some juggling to find EOL for >100b long lines. There
we wont see a bugreport for exceeded line length, but simplifying the code
seemed like a win to me so included that in the updated patch as well.
> Personally, I'd avoid depending on StringInfo.cursor here, as the
> dependency isn't really buying you anything.
Fair enough, I was mainly a bit excited at finally finding a use for .cursor =)
Fixed.
> Also, the need for inserting the pfree into multiple exit paths kind
> of makes me itch. I wonder if we should change the ending code to
> look like
>
> exit:
> fclose(f);
> pfree(linebuf.data);
>
> return result;
>
> and then the early exit spots would be replaced with "result = x;
> goto exit". (Some of them could use "break", but not all, so
> it's probably better to consistently use "goto".)
Agreed, fixed. I was a bit tempted to use something less magic and more
descriptive than result = 3; but in the end opted for keeping changes to one
thing at a time.
cheers ./daniel
Attachment | Content-Type | Size |
---|---|---|
0001-Refactor-pg_service.conf-and-pg_restore-TOC-file-par.patch | application/octet-stream | 7.6 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Daniel Gustafsson | 2020-09-22 12:01:18 | Re: OpenSSL 3.0.0 compatibility |
Previous Message | Ajin Cherian | 2020-09-22 11:47:36 | Re: [HACKERS] logical decoding of two-phase transactions |