pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feedPatch a potential memory leak in describeOneTableDetails()
10+ messages / 4 participants
[nested] [flat]
* Patch a potential memory leak in describeOneTableDetails()
@ 2022-02-18 08:12 wliang@stu.xidian.edu.cn
2022-02-21 08:30 ` Re: Patch a potential memory leak in describeOneTableDetails() Kyotaro Horiguchi <horikyota.ntt@gmail.com>
0 siblings, 1 reply; 10+ messages in thread
From: wliang@stu.xidian.edu.cn @ 2022-02-18 08:12 UTC (permalink / raw)
To: pgsql-hackers@lists.postgresql.org
Hi all,
I find a potential memory leak in PostgresSQL 14.1, which is in the function describeOneTableDetails (./src/bin/psql/describe.c). The bug has been confirmed by an auditor of <pgsql-bugs>.
Specifically, at line 1603, a memory chunk is allocated with pg_strdup and assigned to tableinfo.reloptions. If the branch takes true at line 1621, the execution path will eventually jump to error_return at line 3394. Unfortunately, the memory is not freed when the function describeOneTableDetail() returns. As a result, the memory is leaked.
Similarly, the two memory chunks (tableinfo.reloftype and tableinfo.relam) allocated at line 1605, 1606 and 1612 may also be leaked for the same reason.
1355 bool
1356 describeTableDetails(const char *pattern, bool verbose, bool showSystem)
1357 {
...
1603 tableinfo.reloptions = pg_strdup(PQgetvalue(res, 0, 9));
1604 tableinfo.tablespace = atooid(PQgetvalue(res, 0, 10));
1605 tableinfo.reloftype = (strcmp(PQgetvalue(res, 0, 11), "") != 0) ?
1606 pg_strdup(PQgetvalue(res, 0, 11)) : NULL;
...
1610 if (pset.sversion >= 120000)
1611 tableinfo.relam = PQgetisnull(res, 0, 14) ?
1612 (char *) NULL : pg_strdup(PQgetvalue(res, 0, 14));
1613 else
1614 tableinfo.relam = NULL;
...
1621 if (tableinfo.relkind == RELKIND_SEQUENCE)
1622 {
...
1737 goto error_return; /* not an error, just return early */
1738 }
...
3394 error_return:
3395
3396 /* clean up */
3397 if (printTableInitialized)
3398 printTableCleanup(&cont);
3399 termPQExpBuffer(&buf);
3400 termPQExpBuffer(&title);
3401 termPQExpBuffer(&tmpbuf);
3402
3403 if (view_def)
3404 free(view_def);
3405
3406 if (res)
3407 PQclear(res);
3408
3409 return retval;
3410 }
We believe we can fix the problems by adding corresponding release function before the function returns, such as.
if (tableinfo.reloptions)
pg_free (tableinfo.reloptions);
if (tableinfo.reloftype)
pg_free (tableinfo.reloftype);
if (tableinfo.relam)
pg_free (tableinfo. relam);
I'm looking forward to your reply.
Best,
Wentao
Attachments:
describe.c.patch (text/x-patch, 390B) download
--- describe.c 2022-01-26 16:17:51.353135285 +0800
+++ describe_Patch.c 2022-02-18 15:57:51.099581978 +0800
@@ -3406,6 +3406,13 @@
if (res)
PQclear(res);
+ if (tableinfo.reloptions)
+ pg_free (tableinfo.reloptions);
+ if (tableinfo.reloftype)
+ pg_free (tableinfo.reloftype);
+ if (tableinfo.relam)
+ pg_free (tableinfo. relam);
+
return retval;
}
* Re: Patch a potential memory leak in describeOneTableDetails()
2022-02-18 08:12 Patch a potential memory leak in describeOneTableDetails() wliang@stu.xidian.edu.cn
@ 2022-02-21 08:30 ` Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2022-02-21 09:24 ` Re: Patch a potential memory leak in describeOneTableDetails() Daniel Gustafsson <daniel@yesql.se>
0 siblings, 1 reply; 10+ messages in thread
From: Kyotaro Horiguchi @ 2022-02-21 08:30 UTC (permalink / raw)
To: wliang@stu.xidian.edu.cn; +Cc: pgsql-hackers@lists.postgresql.org
At Fri, 18 Feb 2022 16:12:34 +0800 (GMT+08:00), wliang@stu.xidian.edu.cn wrote in
> I find a potential memory leak in PostgresSQL 14.1, which is in the function describeOneTableDetails (./src/bin/psql/describe.c). The bug has been confirmed by an auditor of <pgsql-bugs>.
>
> Specifically, at line 1603, a memory chunk is allocated with pg_strdup and assigned to tableinfo.reloptions. If the branch takes true at line 1621, the execution path will eventually jump to error_return at line 3394. Unfortunately, the memory is not freed when the function describeOneTableDetail() returns. As a result, the memory is leaked.
>
> Similarly, the two memory chunks (tableinfo.reloftype and tableinfo.relam) allocated at line 1605, 1606 and 1612 may also be leaked for the same reason.
I think it is not potential leaks but real leaks as it accumulates as repeatedly doing \d <table>.
> We believe we can fix the problems by adding corresponding release function before the function returns, such as.
>
> if (tableinfo.reloptions)
> pg_free (tableinfo.reloptions);
> if (tableinfo.reloftype)
> pg_free (tableinfo.reloftype);
> if (tableinfo.relam)
> pg_free (tableinfo. relam);
>
>
> I'm looking forward to your reply.
Good catch, and the fix is logically correct. The results from other
use of pg_strdup() (and pg_malloc) seems to be released properly.
About the patch, we should make a single chunk to do free(). And I
think we don't insert whitespace between function name and the
following left parenthesis.
Since view_def is allocated by pg_strdup(), we might be better use
pg_free() for it for symmetry. footers[0] is allocated using the
frontend-alternative of "palloc()" so should use pfree() instead?
Honestly I'm not sure we need to be so strict on that
correspondence...
So it would be something like this.
inline patch 1: src/bin/psql/describe.c (+9/-3 lines) download inline-1.patch
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index 654ef2d7c3..5da2b61a88 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -3412,7 +3412,13 @@ error_return:
termPQExpBuffer(&tmpbuf);
if (view_def)
- free(view_def);
+ pg_free(view_def);
+ if (tableinfo.reloptions)
+ pg_free(tableinfo.reloptions);
+ if (tableinfo.reloftype)
+ pg_free(tableinfo.reloftype);
+ if (tableinfo.relam)
+ pg_free(tableinfo.relam);
if (res)
PQclear(res);
@@ -3621,8 +3627,8 @@ describeRoles(const char *pattern, bool verbose, bool showSystem)
printTableCleanup(&cont);
for (i = 0; i < nrows; i++)
- free(attr[i]);
- free(attr);
+ pg_free(attr[i]);
+ pg_free(attr);
PQclear(res);
return true;
* Re: Patch a potential memory leak in describeOneTableDetails()
2022-02-18 08:12 Patch a potential memory leak in describeOneTableDetails() wliang@stu.xidian.edu.cn
2022-02-21 08:30 ` Re: Patch a potential memory leak in describeOneTableDetails() Kyotaro Horiguchi <horikyota.ntt@gmail.com>
@ 2022-02-21 09:24 ` Daniel Gustafsson <daniel@yesql.se>
2022-02-22 06:14 ` Re: Patch a potential memory leak in describeOneTableDetails() Kyotaro Horiguchi <horikyota.ntt@gmail.com>
0 siblings, 1 reply; 10+ messages in thread
From: Daniel Gustafsson @ 2022-02-21 09:24 UTC (permalink / raw)
To: Kyotaro Horiguchi <horikyota.ntt@gmail.com>; +Cc: wliang@stu.xidian.edu.cn; pgsql-hackers@lists.postgresql.org
> On 21 Feb 2022, at 09:30, Kyotaro Horiguchi <horikyota.ntt@gmail.com> wrote:
> (However, I got a mysterious -Wmisleading-indentation warning with this..)
>
>> describe.c: In function ‘describeOneTableDetails’:
>> describe.c:3420:5: warning: this ‘if’ clause does not guard... [-Wmisleading-indentation]
>> if (tableinfo.relam)
>> ^~
>> describe.c:3423:2: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the ‘if’
>> if (res)
> ^~
I think it's because you've indented your new code differently from the
existing, such that the if (res) clause is indented equally to the previous
pg_free(tableinfo.relam) call, making it look like they are in the same block:
> + if (tableinfo.relam)
> + pg_free(tableinfo.relam);
>
> if (res)
> PQclear(res);
--
Daniel Gustafsson https://vmware.com/
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: Patch a potential memory leak in describeOneTableDetails()
2022-02-18 08:12 Patch a potential memory leak in describeOneTableDetails() wliang@stu.xidian.edu.cn
2022-02-21 08:30 ` Re: Patch a potential memory leak in describeOneTableDetails() Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2022-02-21 09:24 ` Re: Patch a potential memory leak in describeOneTableDetails() Daniel Gustafsson <daniel@yesql.se>
@ 2022-02-22 06:14 ` Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2022-02-22 08:59 ` Re: Patch a potential memory leak in describeOneTableDetails() Daniel Gustafsson <daniel@yesql.se>
2022-02-22 15:00 ` Re: Patch a potential memory leak in describeOneTableDetails() Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 2 replies; 10+ messages in thread
From: Kyotaro Horiguchi @ 2022-02-22 06:14 UTC (permalink / raw)
To: daniel@yesql.se; +Cc: wliang@stu.xidian.edu.cn; pgsql-hackers@lists.postgresql.org
At Mon, 21 Feb 2022 10:24:23 +0100, Daniel Gustafsson <daniel@yesql.se> wrote in
> I think it's because you've indented your new code differently from the
> existing, such that the if (res) clause is indented equally to the previous
> pg_free(tableinfo.relam) call, making it look like they are in the same block:
Yeah, I (wrongly) thought that they have the same indentation.
> > + if (tableinfo.relam)
> > + pg_free(tableinfo.relam);
> >
> > if (res)
> > PQclear(res);
But actually the lines I added are space-indented but the following
existing line is tab-indented.
Anyway, don't we use the -ftabstop option flag to silence compiler?
The warning is contradicting our convention. The attached adds that
flag.
By the way, the doc says that:
https://www.postgresql.org/docs/14/source-format.html
> The text browsing tools more and less can be invoked as:
> more -x4
> less -x4
> to make them show tabs appropriately.
But AFAICS the "more" command on CentOS doesn't have -x options nor
any option to change tab width and I don't find a document that
suggests its existence on other platforms.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
Attachments:
0001-Fit-compiler-tabstop-to-our-convension.txt (text/plain, 4.4K) download
From 4e3b0c2ce6a2dd60d11100deb2db80ff07455d0d Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Date: Tue, 22 Feb 2022 14:41:10 +0900
Subject: [PATCH] Fit compiler tabstop to our convension
Set -ftabstop to 4 so that -Wmisleading-indentation doesn't makes
false warnings.
---
configure | 92 ++++++++++++++++++++++++++++++++++++++++++++++++++++
configure.ac | 3 ++
2 files changed, 95 insertions(+)
diff --git a/configure b/configure
index f3cb5c2b51..887ed733eb 100755
--- a/configure
+++ b/configure
@@ -6219,6 +6219,98 @@ if test x"$pgac_cv_prog_CXX_cxxflags__fexcess_precision_standard" = x"yes"; then
fi
+ # Fit tab width for -Wmisleading-indentation to our convention
+
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking whether ${CC} supports -ftabstop=4, for CFLAGS" >&5
+$as_echo_n "checking whether ${CC} supports -ftabstop=4, for CFLAGS... " >&6; }
+if ${pgac_cv_prog_CC_cflags__ftabstop_4+:} false; then :
+ $as_echo_n "(cached) " >&6
+else
+ pgac_save_CFLAGS=$CFLAGS
+pgac_save_CC=$CC
+CC=${CC}
+CFLAGS="${CFLAGS} -ftabstop=4"
+ac_save_c_werror_flag=$ac_c_werror_flag
+ac_c_werror_flag=yes
+cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h. */
+
+int
+main ()
+{
+
+ ;
+ return 0;
+}
+_ACEOF
+if ac_fn_c_try_compile "$LINENO"; then :
+ pgac_cv_prog_CC_cflags__ftabstop_4=yes
+else
+ pgac_cv_prog_CC_cflags__ftabstop_4=no
+fi
+rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
+ac_c_werror_flag=$ac_save_c_werror_flag
+CFLAGS="$pgac_save_CFLAGS"
+CC="$pgac_save_CC"
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv_prog_CC_cflags__ftabstop_4" >&5
+$as_echo "$pgac_cv_prog_CC_cflags__ftabstop_4" >&6; }
+if test x"$pgac_cv_prog_CC_cflags__ftabstop_4" = x"yes"; then
+ CFLAGS="${CFLAGS} -ftabstop=4"
+fi
+
+
+ { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether ${CXX} supports -ftabstop=4, for CXXFLAGS" >&5
+$as_echo_n "checking whether ${CXX} supports -ftabstop=4, for CXXFLAGS... " >&6; }
+if ${pgac_cv_prog_CXX_cxxflags__ftabstop_4+:} false; then :
+ $as_echo_n "(cached) " >&6
+else
+ pgac_save_CXXFLAGS=$CXXFLAGS
+pgac_save_CXX=$CXX
+CXX=${CXX}
+CXXFLAGS="${CXXFLAGS} -ftabstop=4"
+ac_save_cxx_werror_flag=$ac_cxx_werror_flag
+ac_cxx_werror_flag=yes
+ac_ext=cpp
+ac_cpp='$CXXCPP $CPPFLAGS'
+ac_compile='$CXX -c $CXXFLAGS $CPPFLAGS conftest.$ac_ext >&5'
+ac_link='$CXX -o conftest$ac_exeext $CXXFLAGS $CPPFLAGS $LDFLAGS conftest.$ac_ext $LIBS >&5'
+ac_compiler_gnu=$ac_cv_cxx_compiler_gnu
+
+cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h. */
+
+int
+main ()
+{
+
+ ;
+ return 0;
+}
+_ACEOF
+if ac_fn_cxx_try_compile "$LINENO"; then :
+ pgac_cv_prog_CXX_cxxflags__ftabstop_4=yes
+else
+ pgac_cv_prog_CXX_cxxflags__ftabstop_4=no
+fi
+rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
+ac_ext=c
+ac_cpp='$CPP $CPPFLAGS'
+ac_compile='$CC -c $CFLAGS $CPPFLAGS conftest.$ac_ext >&5'
+ac_link='$CC -o conftest$ac_exeext $CFLAGS $CPPFLAGS $LDFLAGS conftest.$ac_ext $LIBS >&5'
+ac_compiler_gnu=$ac_cv_c_compiler_gnu
+
+ac_cxx_werror_flag=$ac_save_cxx_werror_flag
+CXXFLAGS="$pgac_save_CXXFLAGS"
+CXX="$pgac_save_CXX"
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv_prog_CXX_cxxflags__ftabstop_4" >&5
+$as_echo "$pgac_cv_prog_CXX_cxxflags__ftabstop_4" >&6; }
+if test x"$pgac_cv_prog_CXX_cxxflags__ftabstop_4" = x"yes"; then
+ CXXFLAGS="${CXXFLAGS} -ftabstop=4"
+fi
+
+
# Optimization flags for specific files that benefit from loop unrolling
{ $as_echo "$as_me:${as_lineno-$LINENO}: checking whether ${CC} supports -funroll-loops, for CFLAGS_UNROLL_LOOPS" >&5
$as_echo_n "checking whether ${CC} supports -funroll-loops, for CFLAGS_UNROLL_LOOPS... " >&6; }
diff --git a/configure.ac b/configure.ac
index 19d1a80367..808d3d268d 100644
--- a/configure.ac
+++ b/configure.ac
@@ -518,6 +518,9 @@ if test "$GCC" = yes -a "$ICC" = no; then
# Disable FP optimizations that cause various errors on gcc 4.5+ or maybe 4.6+
PGAC_PROG_CC_CFLAGS_OPT([-fexcess-precision=standard])
PGAC_PROG_CXX_CFLAGS_OPT([-fexcess-precision=standard])
+ # Fit tab width for -Wmisleading-indentation to our convention
+ PGAC_PROG_CC_CFLAGS_OPT([-ftabstop=4])
+ PGAC_PROG_CXX_CFLAGS_OPT([-ftabstop=4])
# Optimization flags for specific files that benefit from loop unrolling
PGAC_PROG_CC_VAR_OPT(CFLAGS_UNROLL_LOOPS, [-funroll-loops])
# Optimization flags for specific files that benefit from vectorization
--
2.27.0
* Re: Patch a potential memory leak in describeOneTableDetails()
2022-02-18 08:12 Patch a potential memory leak in describeOneTableDetails() wliang@stu.xidian.edu.cn
2022-02-21 08:30 ` Re: Patch a potential memory leak in describeOneTableDetails() Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2022-02-21 09:24 ` Re: Patch a potential memory leak in describeOneTableDetails() Daniel Gustafsson <daniel@yesql.se>
2022-02-22 06:14 ` Re: Patch a potential memory leak in describeOneTableDetails() Kyotaro Horiguchi <horikyota.ntt@gmail.com>
@ 2022-02-22 08:59 ` Daniel Gustafsson <daniel@yesql.se>
2022-02-24 05:44 ` Re: Patch a potential memory leak in describeOneTableDetails() Kyotaro Horiguchi <horikyota.ntt@gmail.com>
1 sibling, 1 reply; 10+ messages in thread
From: Daniel Gustafsson @ 2022-02-22 08:59 UTC (permalink / raw)
To: Kyotaro Horiguchi <horikyota.ntt@gmail.com>; +Cc: wliang@stu.xidian.edu.cn; pgsql-hackers@lists.postgresql.org
> On 22 Feb 2022, at 07:14, Kyotaro Horiguchi <horikyota.ntt@gmail.com> wrote:
> Anyway, don't we use the -ftabstop option flag to silence compiler?
> The warning is contradicting our convention. The attached adds that
> flag.
Isn't this flag contradicting our convention? From the docs section you
reference further down:
"Source code formatting uses 4 column tab spacing, with tabs preserved
(i.e., tabs are not expanded to spaces)."
The -ftabstop option is (to a large extent, not entirely) to warn when tabs and
spaces are mixed creating misleading indentation that the author didn't even
notice due to tabwidth settings? ISTM we are better of getting these warnings
than suppressing them.
> By the way, the doc says that:
>
> https://www.postgresql.org/docs/14/source-format.html
>
>> The text browsing tools more and less can be invoked as:
>> more -x4
>> less -x4
>> to make them show tabs appropriately.
>
> But AFAICS the "more" command on CentOS doesn't have -x options nor
> any option to change tab width and I don't find a document that
> suggests its existence on other platforms.
macOS, FreeBSD and NetBSD both show the less(1) manpage for more(1) which
suggests that more is implemented using less there, and thus supports -x (it
does on macOS). OpenBSD and Solaris more(1) does not show a -x option, but AIX
does have it. Linux in its various flavors doesn't seem to have it.
The section in question was added to our docs 22 years ago, to make it reflect
the current operating systems in use maybe just not mentioning more(1) is the
easiest?:
"The text browsing tool less can be invoked as less -x4 to make it show
tabs appropriately."
Or perhaps remove that section altogether?
--
Daniel Gustafsson https://vmware.com/
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: Patch a potential memory leak in describeOneTableDetails()
2022-02-18 08:12 Patch a potential memory leak in describeOneTableDetails() wliang@stu.xidian.edu.cn
2022-02-21 08:30 ` Re: Patch a potential memory leak in describeOneTableDetails() Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2022-02-21 09:24 ` Re: Patch a potential memory leak in describeOneTableDetails() Daniel Gustafsson <daniel@yesql.se>
2022-02-22 06:14 ` Re: Patch a potential memory leak in describeOneTableDetails() Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2022-02-22 08:59 ` Re: Patch a potential memory leak in describeOneTableDetails() Daniel Gustafsson <daniel@yesql.se>
@ 2022-02-24 05:44 ` Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2022-02-24 06:32 ` Re: Patch a potential memory leak in describeOneTableDetails() Kyotaro Horiguchi <horikyota.ntt@gmail.com>
0 siblings, 1 reply; 10+ messages in thread
From: Kyotaro Horiguchi @ 2022-02-24 05:44 UTC (permalink / raw)
To: daniel@yesql.se; +Cc: wliang@stu.xidian.edu.cn; pgsql-hackers@lists.postgresql.org
At Tue, 22 Feb 2022 09:59:09 +0100, Daniel Gustafsson <daniel@yesql.se> wrote in
> > On 22 Feb 2022, at 07:14, Kyotaro Horiguchi <horikyota.ntt@gmail.com> wrote:
>
> > Anyway, don't we use the -ftabstop option flag to silence compiler?
> > The warning is contradicting our convention. The attached adds that
> > flag.
>
> Isn't this flag contradicting our convention? From the docs section you
> reference further down:
>
> "Source code formatting uses 4 column tab spacing, with tabs preserved
> (i.e., tabs are not expanded to spaces)."
Ugg. Right.
> The -ftabstop option is (to a large extent, not entirely) to warn when tabs and
> spaces are mixed creating misleading indentation that the author didn't even
> notice due to tabwidth settings? ISTM we are better of getting these warnings
> than suppressing them.
At Tue, 22 Feb 2022 10:00:46 -0500, Tom Lane <tgl@sss.pgh.pa.us> wrote in
> No, we use pgindent to not have such inconsistent indentation.
Understood. Thaks for clarification.
> > By the way, the doc says that:
> >
> > https://www.postgresql.org/docs/14/source-format.html
> >
> >> The text browsing tools more and less can be invoked as:
> >> more -x4
> >> less -x4
> >> to make them show tabs appropriately.
> >
> > But AFAICS the "more" command on CentOS doesn't have -x options nor
> > any option to change tab width and I don't find a document that
> > suggests its existence on other platforms.
>
> macOS, FreeBSD and NetBSD both show the less(1) manpage for more(1) which
> suggests that more is implemented using less there, and thus supports -x (it
> does on macOS). OpenBSD and Solaris more(1) does not show a -x option, but AIX
> does have it. Linux in its various flavors doesn't seem to have it.
Thanks for the explanation.
> The section in question was added to our docs 22 years ago, to make it reflect
> the current operating systems in use maybe just not mentioning more(1) is the
> easiest?:
>
> "The text browsing tool less can be invoked as less -x4 to make it show
> tabs appropriately."
>
> Or perhaps remove that section altogether?
I think the former is better.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: Patch a potential memory leak in describeOneTableDetails()
2022-02-18 08:12 Patch a potential memory leak in describeOneTableDetails() wliang@stu.xidian.edu.cn
2022-02-21 08:30 ` Re: Patch a potential memory leak in describeOneTableDetails() Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2022-02-21 09:24 ` Re: Patch a potential memory leak in describeOneTableDetails() Daniel Gustafsson <daniel@yesql.se>
2022-02-22 06:14 ` Re: Patch a potential memory leak in describeOneTableDetails() Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2022-02-22 08:59 ` Re: Patch a potential memory leak in describeOneTableDetails() Daniel Gustafsson <daniel@yesql.se>
2022-02-24 05:44 ` Re: Patch a potential memory leak in describeOneTableDetails() Kyotaro Horiguchi <horikyota.ntt@gmail.com>
@ 2022-02-24 06:32 ` Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2022-02-24 13:55 ` Re: Patch a potential memory leak in describeOneTableDetails() Daniel Gustafsson <daniel@yesql.se>
0 siblings, 1 reply; 10+ messages in thread
From: Kyotaro Horiguchi @ 2022-02-24 06:32 UTC (permalink / raw)
To: daniel@yesql.se; +Cc: wliang@stu.xidian.edu.cn; pgsql-hackers@lists.postgresql.org
At Thu, 24 Feb 2022 14:44:56 +0900 (JST), Kyotaro Horiguchi <horikyota.ntt@gmail.com> wrote in
> At Tue, 22 Feb 2022 09:59:09 +0100, Daniel Gustafsson <daniel@yesql.se> wrote in
> > The section in question was added to our docs 22 years ago, to make it reflect
> > the current operating systems in use maybe just not mentioning more(1) is the
> > easiest?:
> >
> > "The text browsing tool less can be invoked as less -x4 to make it show
> > tabs appropriately."
> >
> > Or perhaps remove that section altogether?
>
> I think the former is better.
So the attached does that.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
Attachments:
0001-Fix-out-of-date-description-mentioning-the-command-m.txt (text/plain, 1.1K) download
From fd833aee1756364f669a3d478763299ce6d3a747 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Date: Thu, 24 Feb 2022 15:18:52 +0900
Subject: [PATCH] Fix out-of-date description mentioning the command more
In most of the operating systems in use, "more" is implemented using
"less", or otherwise "more" does not have an -x option. Remove the
descriptions related to the more command to make it reflect the
current world better.
---
doc/src/sgml/sources.sgml | 9 ++-------
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/doc/src/sgml/sources.sgml b/doc/src/sgml/sources.sgml
index 1b77efb087..7a6e8a951f 100644
--- a/doc/src/sgml/sources.sgml
+++ b/doc/src/sgml/sources.sgml
@@ -76,13 +76,8 @@
</para>
<para>
- The text browsing tools <application>more</application> and
- <application>less</application> can be invoked as:
-<programlisting>
-more -x4
-less -x4
-</programlisting>
- to make them show tabs appropriately.
+ The text browsing tool <application>less</application> can be invoked
+ as <quote>less -x4</quote> to make it show tabs appropriately.
</para>
</sect1>
--
2.27.0
* Re: Patch a potential memory leak in describeOneTableDetails()
2022-02-18 08:12 Patch a potential memory leak in describeOneTableDetails() wliang@stu.xidian.edu.cn
2022-02-21 08:30 ` Re: Patch a potential memory leak in describeOneTableDetails() Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2022-02-21 09:24 ` Re: Patch a potential memory leak in describeOneTableDetails() Daniel Gustafsson <daniel@yesql.se>
2022-02-22 06:14 ` Re: Patch a potential memory leak in describeOneTableDetails() Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2022-02-22 08:59 ` Re: Patch a potential memory leak in describeOneTableDetails() Daniel Gustafsson <daniel@yesql.se>
2022-02-24 05:44 ` Re: Patch a potential memory leak in describeOneTableDetails() Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2022-02-24 06:32 ` Re: Patch a potential memory leak in describeOneTableDetails() Kyotaro Horiguchi <horikyota.ntt@gmail.com>
@ 2022-02-24 13:55 ` Daniel Gustafsson <daniel@yesql.se>
2022-04-19 12:35 ` Re: Patch a potential memory leak in describeOneTableDetails() Daniel Gustafsson <daniel@yesql.se>
0 siblings, 1 reply; 10+ messages in thread
From: Daniel Gustafsson @ 2022-02-24 13:55 UTC (permalink / raw)
To: Kyotaro Horiguchi <horikyota.ntt@gmail.com>; +Cc: wliang@stu.xidian.edu.cn; pgsql-hackers@lists.postgresql.org
> On 24 Feb 2022, at 07:32, Kyotaro Horiguchi <horikyota.ntt@gmail.com> wrote:
>
> At Thu, 24 Feb 2022 14:44:56 +0900 (JST), Kyotaro Horiguchi <horikyota.ntt@gmail.com> wrote in
>> At Tue, 22 Feb 2022 09:59:09 +0100, Daniel Gustafsson <daniel@yesql.se> wrote in
>>> The section in question was added to our docs 22 years ago, to make it reflect
>>> the current operating systems in use maybe just not mentioning more(1) is the
>>> easiest?:
>>>
>>> "The text browsing tool less can be invoked as less -x4 to make it show
>>> tabs appropriately."
>>>
>>> Or perhaps remove that section altogether?
>>
>> I think the former is better.
>
> So the attached does that.
I think this is reasonable, since it's pretty clear that more(1) supporting
"-x" is fairly rare. I'll await others commenting.
--
Daniel Gustafsson https://vmware.com/
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: Patch a potential memory leak in describeOneTableDetails()
2022-02-18 08:12 Patch a potential memory leak in describeOneTableDetails() wliang@stu.xidian.edu.cn
2022-02-21 08:30 ` Re: Patch a potential memory leak in describeOneTableDetails() Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2022-02-21 09:24 ` Re: Patch a potential memory leak in describeOneTableDetails() Daniel Gustafsson <daniel@yesql.se>
2022-02-22 06:14 ` Re: Patch a potential memory leak in describeOneTableDetails() Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2022-02-22 08:59 ` Re: Patch a potential memory leak in describeOneTableDetails() Daniel Gustafsson <daniel@yesql.se>
2022-02-24 05:44 ` Re: Patch a potential memory leak in describeOneTableDetails() Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2022-02-24 06:32 ` Re: Patch a potential memory leak in describeOneTableDetails() Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2022-02-24 13:55 ` Re: Patch a potential memory leak in describeOneTableDetails() Daniel Gustafsson <daniel@yesql.se>
@ 2022-04-19 12:35 ` Daniel Gustafsson <daniel@yesql.se>
0 siblings, 0 replies; 10+ messages in thread
From: Daniel Gustafsson @ 2022-04-19 12:35 UTC (permalink / raw)
To: Kyotaro Horiguchi <horikyota.ntt@gmail.com>; +Cc: wliang@stu.xidian.edu.cn; pgsql-hackers@lists.postgresql.org
> On 24 Feb 2022, at 14:55, Daniel Gustafsson <daniel@yesql.se> wrote:
>> On 24 Feb 2022, at 07:32, Kyotaro Horiguchi <horikyota.ntt@gmail.com> wrote:
>> So the attached does that.
>
> I think this is reasonable, since it's pretty clear that more(1) supporting
> "-x" is fairly rare. I'll await others commenting.
The original patch which prompted this got a bit buried, the attached 0001
contains that fix (using free() which matches the surrounding code) and 0002
has the documentation fixes for the more/less -x command.
--
Daniel Gustafsson https://vmware.com/
Attachments: download all (2 files, .tar.gz)
v2-0002-Fix-out-of-date-description-mentioning-the-comman.patch (application/octet-stream, 1.3K) download
From d64df9af0e483fba2d01f989abadf260d9b05967 Mon Sep 17 00:00:00 2001
From: Daniel Gustafsson <daniel@yesql.se>
Date: Tue, 19 Apr 2022 14:25:54 +0200
Subject: [PATCH v2 2/2] Fix out-of-date description mentioning the command
more
In operating systems where the 'more' command isn't implemented using
the 'less' command, it's not all too common for 'more' to have the -x
option. Reword the source formatting paragraph to better reflect the
current situation and just mention 'less' as capable of -x.
Author: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Discussion: https://postgr.es/m/20220222.151401.1451845726756615483.horikyota.ntt@gmail.com
---
doc/src/sgml/sources.sgml | 9 ++-------
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/doc/src/sgml/sources.sgml b/doc/src/sgml/sources.sgml
index 2d01efc950..a29e09e466 100644
--- a/doc/src/sgml/sources.sgml
+++ b/doc/src/sgml/sources.sgml
@@ -76,13 +76,8 @@
</para>
<para>
- The text browsing tools <application>more</application> and
- <application>less</application> can be invoked as:
-<programlisting>
-more -x4
-less -x4
-</programlisting>
- to make them show tabs appropriately.
+ The text browsing tool <application>less</application> can be invoked as:
+ <literal>less -x4</literal> to show tabs appropriately.
</para>
</sect1>
--
2.32.0 (Apple Git-132)
=
v2-0001-Fix-small-memory-leak-in-psql-d.patch (application/octet-stream, 1.1K) download
From 24d907940d2fc8f2850b0ba57ebd98c5d7d8f667 Mon Sep 17 00:00:00 2001
From: Daniel Gustafsson <daniel@yesql.se>
Date: Tue, 19 Apr 2022 13:45:32 +0200
Subject: [PATCH v2 1/2] Fix small memory leak in psql \d
The memory allocated to store table information wasn't freed causing
a small leak. Fix by freeing like we do for other allocations in the
same codepath.
Author: Wentao Liang <wliang@stu.xidian.edu.cn>
Reviewed-by: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Discussion: https://postgr.es/m/38da8aa3.14ec.17f0be3d3ad.Coremail.wliang@stu.xidian.edu.cn
---
src/bin/psql/describe.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index 583817b0cc..6c8aa289c1 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -3452,6 +3452,13 @@ error_return:
if (view_def)
free(view_def);
+ if (tableinfo.reloptions)
+ free(tableinfo.reloptions);
+ if (tableinfo.reloftype)
+ free(tableinfo.reloftype);
+ if (tableinfo.relam)
+ free(tableinfo.relam);
+
if (res)
PQclear(res);
--
2.32.0 (Apple Git-132)
=
* Re: Patch a potential memory leak in describeOneTableDetails()
2022-02-18 08:12 Patch a potential memory leak in describeOneTableDetails() wliang@stu.xidian.edu.cn
2022-02-21 08:30 ` Re: Patch a potential memory leak in describeOneTableDetails() Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2022-02-21 09:24 ` Re: Patch a potential memory leak in describeOneTableDetails() Daniel Gustafsson <daniel@yesql.se>
2022-02-22 06:14 ` Re: Patch a potential memory leak in describeOneTableDetails() Kyotaro Horiguchi <horikyota.ntt@gmail.com>
@ 2022-02-22 15:00 ` Tom Lane <tgl@sss.pgh.pa.us>
1 sibling, 0 replies; 10+ messages in thread
From: Tom Lane @ 2022-02-22 15:00 UTC (permalink / raw)
To: Kyotaro Horiguchi <horikyota.ntt@gmail.com>; +Cc: daniel@yesql.se; wliang@stu.xidian.edu.cn; pgsql-hackers@lists.postgresql.org
Kyotaro Horiguchi <horikyota.ntt@gmail.com> writes:
> Anyway, don't we use the -ftabstop option flag to silence compiler?
> The warning is contradicting our convention. The attached adds that
> flag.
No, we use pgindent to not have such inconsistent indentation.
regards, tom lane
^ permalink raw reply [nested|flat] 10+ messages in thread
end of thread, other threads:[~2022-04-19 12:35 UTC | newest]
Thread overview: 10+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2022-02-18 08:12 Patch a potential memory leak in describeOneTableDetails() wliang@stu.xidian.edu.cn
2022-02-21 08:30 ` Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2022-02-21 09:24 ` Daniel Gustafsson <daniel@yesql.se>
2022-02-22 06:14 ` Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2022-02-22 08:59 ` Daniel Gustafsson <daniel@yesql.se>
2022-02-24 05:44 ` Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2022-02-24 06:32 ` Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2022-02-24 13:55 ` Daniel Gustafsson <daniel@yesql.se>
2022-04-19 12:35 ` Daniel Gustafsson <daniel@yesql.se>
2022-02-22 15:00 ` Tom Lane <tgl@sss.pgh.pa.us>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox