public inbox for [email protected]
help / color / mirror / Atom feed[PATCH] pgindent truncates last line of files missing a trailing newline
7+ messages / 5 participants
[nested] [flat]
* [PATCH] pgindent truncates last line of files missing a trailing newline
@ 2026-02-16 14:09 Akshay Joshi <[email protected]>
2026-02-16 16:28 ` Re: [PATCH] pgindent truncates last line of files missing a trailing newline Andrew Dunstan <[email protected]>
0 siblings, 1 reply; 7+ messages in thread
From: Akshay Joshi @ 2026-02-16 14:09 UTC (permalink / raw)
To: pgsql-hackers
Hi Hackers,
I have encountered a bug in "*src/tools/pg_bsd_indent/lexi.c"* where
pgindent was incorrectly dropping the final line of a file if that line did
not end with a newline character (\n). This occurred because the EOF logic
triggered a termination of the processing loop before the remaining buffer
was flushed to the output.
*Steps to Reproduce:*
1. Create a *header/c* file with some content but no newline at the very
end.
2. Run pgindent on those files.
3. Check the file content. The file is now empty or missing the last
line.
This patch ensures that had_eof states do not bypass the final buffer
processing, preserving the integrity of the source code regardless of
trailing whitespace.
Akshay Joshi
Principal Engineer | Engineering Manager | pgAdmin Hacker
enterprisedb.com
* Blog*: https://www.enterprisedb.com/akshay-joshi
* GitHub*: https://github.com/akshay-joshi
* LinkedIn*: https:// <http://goog_373708537;
www.linkedin.com/in/akshay-joshi-a9317b14
Attachments:
[application/octet-stream] 0001-Fix-pgindent-truncates-last-line-of-files.patch (2.9K, 3-0001-Fix-pgindent-truncates-last-line-of-files.patch)
download | inline diff:
From 5bb56e1e603a8ea98a5f13b7abdaa5f252b3170f Mon Sep 17 00:00:00 2001
From: Akshay Joshi <[email protected]>
Date: Mon, 16 Feb 2026 16:39:08 +0530
Subject: [PATCH] Fix pgindent truncates last line of files missing a trailing
newline.
pgindent was incorrectly dropping the final line of a file if that line
did not end with a newline character (\n). This occurred because the EOF
logic triggered a termination of the processing loop before the remaining
buffer was flushed to the output.
This patch ensures that had_eof states do not bypass the final buffer
processing, preserving the integrity of the source code regardless of trailing
whitespace.
---
src/tools/pg_bsd_indent/lexi.c | 4 +++-
src/tools/pg_bsd_indent/tests/no_trailing_newline.0 | 4 ++++
src/tools/pg_bsd_indent/tests/no_trailing_newline.0.stdout | 6 ++++++
3 files changed, 13 insertions(+), 1 deletion(-)
create mode 100644 src/tools/pg_bsd_indent/tests/no_trailing_newline.0
create mode 100644 src/tools/pg_bsd_indent/tests/no_trailing_newline.0.stdout
diff --git a/src/tools/pg_bsd_indent/lexi.c b/src/tools/pg_bsd_indent/lexi.c
index 943bf7ce6b0..3e46309b257 100644
--- a/src/tools/pg_bsd_indent/lexi.c
+++ b/src/tools/pg_bsd_indent/lexi.c
@@ -219,6 +219,7 @@ lexi(struct parser_state *state)
* forces a following operator to be unary */
int code; /* internal code to be returned */
char qchar; /* the delimiter character for a string */
+ int eof_before_fill; /* had_eof before fill_buffer() call */
e_token = s_token; /* point to start of place to save token */
unary_delim = false;
@@ -446,6 +447,7 @@ lexi(struct parser_state *state)
*e_token++ = *buf_ptr; /* if it is only a one-character token, it is
* moved here */
*e_token = '\0';
+ eof_before_fill = had_eof;
if (++buf_ptr >= buf_end)
fill_buffer();
@@ -453,7 +455,7 @@ lexi(struct parser_state *state)
case '\n':
unary_delim = state->last_u_d;
state->last_nl = true; /* remember that we just had a newline */
- code = (had_eof ? 0 : newline);
+ code = (eof_before_fill ? 0 : newline);
/*
* if data has been exhausted, the newline is a dummy, and we should
diff --git a/src/tools/pg_bsd_indent/tests/no_trailing_newline.0 b/src/tools/pg_bsd_indent/tests/no_trailing_newline.0
new file mode 100644
index 00000000000..3233990e11f
--- /dev/null
+++ b/src/tools/pg_bsd_indent/tests/no_trailing_newline.0
@@ -0,0 +1,4 @@
+/* Test: file without trailing newline */
+void test(void) {
+int x = 1;
+}
\ No newline at end of file
diff --git a/src/tools/pg_bsd_indent/tests/no_trailing_newline.0.stdout b/src/tools/pg_bsd_indent/tests/no_trailing_newline.0.stdout
new file mode 100644
index 00000000000..c632d62f16c
--- /dev/null
+++ b/src/tools/pg_bsd_indent/tests/no_trailing_newline.0.stdout
@@ -0,0 +1,6 @@
+/* Test: file without trailing newline */
+void
+test(void)
+{
+ int x = 1;
+}
--
2.51.0
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: [PATCH] pgindent truncates last line of files missing a trailing newline
2026-02-16 14:09 [PATCH] pgindent truncates last line of files missing a trailing newline Akshay Joshi <[email protected]>
@ 2026-02-16 16:28 ` Andrew Dunstan <[email protected]>
2026-02-16 16:50 ` Re: [PATCH] pgindent truncates last line of files missing a trailing newline Tom Lane <[email protected]>
0 siblings, 1 reply; 7+ messages in thread
From: Andrew Dunstan @ 2026-02-16 16:28 UTC (permalink / raw)
To: Akshay Joshi <[email protected]>; pgsql-hackers
On 2026-02-16 Mo 9:09 AM, Akshay Joshi wrote:
> Hi Hackers,
>
> I have encountered a bug in "*src/tools/pg_bsd_indent/lexi.c"* where
> pgindent was incorrectly dropping the final line of a file if that
> line did not end with a newline character (\n). This occurred because
> the EOF logic triggered a termination of the processing loop before
> the remaining buffer was flushed to the output.
>
> *Steps to Reproduce:*
>
> 1. Create a *header/c* file with some content but no newline at the
> very end.
> 2. Run pgindent on those files.
> 3. Check the file content. The file is now empty or missing the last
> line.
>
> This patch ensures that had_eof states do not bypass the final buffer
> processing, preserving the integrity of the source code regardless of
> trailing whitespace.
Yeah. I wonder if we shouldn't be just ensuring that there is a line
feed at the end of the buffer, i.e. add one if it's not there. We
shouldn't be adding files without a trailing LF, and ensuring there is
one seems like a reasonable thing to do in pg_bsd_indent.
cheers
andrew
--
Andrew Dunstan
EDB:https://www.enterprisedb.com
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: [PATCH] pgindent truncates last line of files missing a trailing newline
2026-02-16 14:09 [PATCH] pgindent truncates last line of files missing a trailing newline Akshay Joshi <[email protected]>
2026-02-16 16:28 ` Re: [PATCH] pgindent truncates last line of files missing a trailing newline Andrew Dunstan <[email protected]>
@ 2026-02-16 16:50 ` Tom Lane <[email protected]>
2026-02-17 02:20 ` Re: [PATCH] pgindent truncates last line of files missing a trailing newline Ashutosh Bapat <[email protected]>
0 siblings, 1 reply; 7+ messages in thread
From: Tom Lane @ 2026-02-16 16:50 UTC (permalink / raw)
To: Andrew Dunstan <[email protected]>; +Cc: Akshay Joshi <[email protected]>; pgsql-hackers
Andrew Dunstan <[email protected]> writes:
> Yeah. I wonder if we shouldn't be just ensuring that there is a line
> feed at the end of the buffer, i.e. add one if it's not there. We
> shouldn't be adding files without a trailing LF, and ensuring there is
> one seems like a reasonable thing to do in pg_bsd_indent.
+1
regards, tom lane
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: [PATCH] pgindent truncates last line of files missing a trailing newline
2026-02-16 14:09 [PATCH] pgindent truncates last line of files missing a trailing newline Akshay Joshi <[email protected]>
2026-02-16 16:28 ` Re: [PATCH] pgindent truncates last line of files missing a trailing newline Andrew Dunstan <[email protected]>
2026-02-16 16:50 ` Re: [PATCH] pgindent truncates last line of files missing a trailing newline Tom Lane <[email protected]>
@ 2026-02-17 02:20 ` Ashutosh Bapat <[email protected]>
2026-02-17 07:54 ` Re: [PATCH] pgindent truncates last line of files missing a trailing newline Akshay Joshi <[email protected]>
0 siblings, 1 reply; 7+ messages in thread
From: Ashutosh Bapat @ 2026-02-17 02:20 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Andrew Dunstan <[email protected]>; Akshay Joshi <[email protected]>; pgsql-hackers
On Mon, Feb 16, 2026 at 10:20 PM Tom Lane <[email protected]> wrote:
>
> Andrew Dunstan <[email protected]> writes:
> > Yeah. I wonder if we shouldn't be just ensuring that there is a line
> > feed at the end of the buffer, i.e. add one if it's not there. We
> > shouldn't be adding files without a trailing LF, and ensuring there is
> > one seems like a reasonable thing to do in pg_bsd_indent.
>
> +1
+1. That will allow me to remove some code from my patch preparation
script. Ref. [1]
[1] https://www.postgresql.org/message-id/flat/CAExHW5v8u7-2H2LqWP3ybhh5GnAVVeCOYuTfkg9pmdnrLwAtNA%40mai...
--
Best Wishes,
Ashutosh Bapat
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: [PATCH] pgindent truncates last line of files missing a trailing newline
2026-02-16 14:09 [PATCH] pgindent truncates last line of files missing a trailing newline Akshay Joshi <[email protected]>
2026-02-16 16:28 ` Re: [PATCH] pgindent truncates last line of files missing a trailing newline Andrew Dunstan <[email protected]>
2026-02-16 16:50 ` Re: [PATCH] pgindent truncates last line of files missing a trailing newline Tom Lane <[email protected]>
2026-02-17 02:20 ` Re: [PATCH] pgindent truncates last line of files missing a trailing newline Ashutosh Bapat <[email protected]>
@ 2026-02-17 07:54 ` Akshay Joshi <[email protected]>
2026-03-27 19:08 ` Re: [PATCH] pgindent truncates last line of files missing a trailing newline Andrew Dunstan <[email protected]>
0 siblings, 1 reply; 7+ messages in thread
From: Akshay Joshi @ 2026-02-17 07:54 UTC (permalink / raw)
To: Ashutosh Bapat <[email protected]>; +Cc: Tom Lane <[email protected]>; Andrew Dunstan <[email protected]>; pgsql-hackers
I have addressed the review comments from Andrew.
Attached is the v2 patch, ready for review.
On Tue, Feb 17, 2026 at 7:50 AM Ashutosh Bapat
<[email protected]> wrote:
>
> On Mon, Feb 16, 2026 at 10:20 PM Tom Lane <[email protected]> wrote:
> >
> > Andrew Dunstan <[email protected]> writes:
> > > Yeah. I wonder if we shouldn't be just ensuring that there is a line
> > > feed at the end of the buffer, i.e. add one if it's not there. We
> > > shouldn't be adding files without a trailing LF, and ensuring there is
> > > one seems like a reasonable thing to do in pg_bsd_indent.
> >
> > +1
>
> +1. That will allow me to remove some code from my patch preparation
> script. Ref. [1]
>
> [1] https://www.postgresql.org/message-id/flat/CAExHW5v8u7-2H2LqWP3ybhh5GnAVVeCOYuTfkg9pmdnrLwAtNA%40mai...
>
> --
> Best Wishes,
> Ashutosh Bapat
Attachments:
[application/octet-stream] v2-0001-Fix-pgindent-truncates-last-line-of-files.patch (2.7K, 2-v2-0001-Fix-pgindent-truncates-last-line-of-files.patch)
download | inline diff:
From f1f4c2886e07d8a376c34cd958eea1534ee5f96e Mon Sep 17 00:00:00 2001
From: Akshay Joshi <[email protected]>
Date: Mon, 16 Feb 2026 16:39:08 +0530
Subject: [PATCH] Fix pgindent to ensure files end with a trailing newline.
If a source file is missing a trailing newline, pg_bsd_indent now adds
one. The fill_buffer() function already adds a synthetic newline at EOF;
this change ensures that newline is emitted to the output rather than
being suppressed.
Author: Akshay Joshi <[email protected]>
Reviewed-by: Andrew Dunstan <[email protected]>
---
src/tools/pg_bsd_indent/lexi.c | 29 +++++++++++++++----
.../pg_bsd_indent/tests/no_trailing_newline.0 | 4 +++
.../tests/no_trailing_newline.0.stdout | 6 ++++
3 files changed, 33 insertions(+), 6 deletions(-)
create mode 100644 src/tools/pg_bsd_indent/tests/no_trailing_newline.0
create mode 100644 src/tools/pg_bsd_indent/tests/no_trailing_newline.0.stdout
diff --git a/src/tools/pg_bsd_indent/lexi.c b/src/tools/pg_bsd_indent/lexi.c
index 943bf7ce6b0..b026fef6fca 100644
--- a/src/tools/pg_bsd_indent/lexi.c
+++ b/src/tools/pg_bsd_indent/lexi.c
@@ -453,12 +453,29 @@ lexi(struct parser_state *state)
case '\n':
unary_delim = state->last_u_d;
state->last_nl = true; /* remember that we just had a newline */
- code = (had_eof ? 0 : newline);
-
- /*
- * if data has been exhausted, the newline is a dummy, and we should
- * return code to stop
- */
+ {
+ /*
+ * Ensure we output exactly one newline at EOF: either the file's real
+ * trailing newline, or the synthetic one added by fill_buffer() if the
+ * file was missing one.
+ */
+ static bool final_newline_done = false;
+
+ if (had_eof)
+ {
+ if (final_newline_done)
+ code = 0; /* already emitted final newline, stop now */
+ else
+ {
+ code = newline;
+ final_newline_done = true;
+ }
+ }
+ else
+ {
+ code = newline;
+ }
+ }
break;
case '\'': /* start of quoted character */
diff --git a/src/tools/pg_bsd_indent/tests/no_trailing_newline.0 b/src/tools/pg_bsd_indent/tests/no_trailing_newline.0
new file mode 100644
index 00000000000..3233990e11f
--- /dev/null
+++ b/src/tools/pg_bsd_indent/tests/no_trailing_newline.0
@@ -0,0 +1,4 @@
+/* Test: file without trailing newline */
+void test(void) {
+int x = 1;
+}
\ No newline at end of file
diff --git a/src/tools/pg_bsd_indent/tests/no_trailing_newline.0.stdout b/src/tools/pg_bsd_indent/tests/no_trailing_newline.0.stdout
new file mode 100644
index 00000000000..c632d62f16c
--- /dev/null
+++ b/src/tools/pg_bsd_indent/tests/no_trailing_newline.0.stdout
@@ -0,0 +1,6 @@
+/* Test: file without trailing newline */
+void
+test(void)
+{
+ int x = 1;
+}
--
2.51.0
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: [PATCH] pgindent truncates last line of files missing a trailing newline
2026-02-16 14:09 [PATCH] pgindent truncates last line of files missing a trailing newline Akshay Joshi <[email protected]>
2026-02-16 16:28 ` Re: [PATCH] pgindent truncates last line of files missing a trailing newline Andrew Dunstan <[email protected]>
2026-02-16 16:50 ` Re: [PATCH] pgindent truncates last line of files missing a trailing newline Tom Lane <[email protected]>
2026-02-17 02:20 ` Re: [PATCH] pgindent truncates last line of files missing a trailing newline Ashutosh Bapat <[email protected]>
2026-02-17 07:54 ` Re: [PATCH] pgindent truncates last line of files missing a trailing newline Akshay Joshi <[email protected]>
@ 2026-03-27 19:08 ` Andrew Dunstan <[email protected]>
2026-03-29 02:26 ` Re: [PATCH] pgindent truncates last line of files missing a trailing newline Bruce Momjian <[email protected]>
0 siblings, 1 reply; 7+ messages in thread
From: Andrew Dunstan @ 2026-03-27 19:08 UTC (permalink / raw)
To: Tom Lane <[email protected]>; Akshay Joshi <[email protected]>; +Cc: Ashutosh Bapat <[email protected]>; pgsql-hackers
On 2026-03-26 Th 12:29 PM, Tom Lane wrote:
> Akshay Joshi<[email protected]> writes:
>> I have addressed the review comments from Andrew.
>> Attached is the v2 patch, ready for review.
> I'm not sure I want to expend the brain cells to figure out whether
> this is a correct/complete patch. If pg_bsd_indent were less of an
> undercommented spaghetti-code nightmare, maybe fixing it here would
> be reasonable. But as things stand, why don't we just fix this in
> the perl wrapper, as attached?
Well, I thought we were trying to reduce the fixups we did in pgindent.
However, I take your point about the ugly nature of the pg_bsd_indent
code. So I'm ok with this fix, which is quite straightforward.
>
> (In any case, I'm not in favor of adding a test case, because that
> would require putting a trailing-newline-less .c file into our tree.
> At best there would be a permanent hazard of something "fixing"
> the file.)
>
>
fair point.
cheers
andrew
--
Andrew Dunstan
EDB:https://www.enterprisedb.com
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: [PATCH] pgindent truncates last line of files missing a trailing newline
2026-02-16 14:09 [PATCH] pgindent truncates last line of files missing a trailing newline Akshay Joshi <[email protected]>
2026-02-16 16:28 ` Re: [PATCH] pgindent truncates last line of files missing a trailing newline Andrew Dunstan <[email protected]>
2026-02-16 16:50 ` Re: [PATCH] pgindent truncates last line of files missing a trailing newline Tom Lane <[email protected]>
2026-02-17 02:20 ` Re: [PATCH] pgindent truncates last line of files missing a trailing newline Ashutosh Bapat <[email protected]>
2026-02-17 07:54 ` Re: [PATCH] pgindent truncates last line of files missing a trailing newline Akshay Joshi <[email protected]>
2026-03-27 19:08 ` Re: [PATCH] pgindent truncates last line of files missing a trailing newline Andrew Dunstan <[email protected]>
@ 2026-03-29 02:26 ` Bruce Momjian <[email protected]>
0 siblings, 0 replies; 7+ messages in thread
From: Bruce Momjian @ 2026-03-29 02:26 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Andrew Dunstan <[email protected]>; Akshay Joshi <[email protected]>; Ashutosh Bapat <[email protected]>; pgsql-hackers
On Fri, Mar 27, 2026 at 03:28:16PM -0400, Tom Lane wrote:
> Andrew Dunstan <[email protected]> writes:
> > On 2026-03-26 Th 12:29 PM, Tom Lane wrote:
> >> I'm not sure I want to expend the brain cells to figure out whether
> >> this is a correct/complete patch. If pg_bsd_indent were less of an
> >> undercommented spaghetti-code nightmare, maybe fixing it here would
> >> be reasonable. But as things stand, why don't we just fix this in
> >> the perl wrapper, as attached?
>
> > Well, I thought we were trying to reduce the fixups we did in pgindent.
>
> Yeah, other things being equal I too would prefer to fix it within
> pg_bsd_indent. But other things are not equal: the complexity
> and reviewability of the proposed v2 patch are not comparable to
> a one-liner fix in the wrapper.
I think the only way forward if we want to continue modifying
pg_bsd_indent is to rename most of the variables and add some sanity to
the code.
--
Bruce Momjian <[email protected]> https://momjian.us
EDB https://enterprisedb.com
Do not let urgent matters crowd out time for investment in the future.
^ permalink raw reply [nested|flat] 7+ messages in thread
end of thread, other threads:[~2026-03-29 02:26 UTC | newest]
Thread overview: 7+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-02-16 14:09 [PATCH] pgindent truncates last line of files missing a trailing newline Akshay Joshi <[email protected]>
2026-02-16 16:28 ` Andrew Dunstan <[email protected]>
2026-02-16 16:50 ` Tom Lane <[email protected]>
2026-02-17 02:20 ` Ashutosh Bapat <[email protected]>
2026-02-17 07:54 ` Akshay Joshi <[email protected]>
2026-03-27 19:08 ` Andrew Dunstan <[email protected]>
2026-03-29 02:26 ` Bruce Momjian <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox