get_progname() should not be const char *?

Started by Phil Sorberalmost 13 years ago6 messages
#1Phil Sorber
phil@omniti.com

get_progname() returns a strdup()'d value. Shouldn't it then be simply
char * and not const char *? Otherwise free() complains loudly without
a cast.

diff --git a/src/include/port.h b/src/include/port.h
new file mode 100644
index 99d3a9b..2d6a435
*** a/src/include/port.h
--- b/src/include/port.h
*************** extern void make_native_path(char *path)
*** 45,51 ****
  extern bool path_contains_parent_reference(const char *path);
  extern bool path_is_relative_and_below_cwd(const char *path);
  extern bool path_is_prefix_of_path(const char *path1, const char *path2);
! extern const char *get_progname(const char *argv0);
  extern void get_share_path(const char *my_exec_path, char *ret_path);
  extern void get_etc_path(const char *my_exec_path, char *ret_path);
  extern void get_include_path(const char *my_exec_path, char *ret_path);
--- 45,51 ----
  extern bool path_contains_parent_reference(const char *path);
  extern bool path_is_relative_and_below_cwd(const char *path);
  extern bool path_is_prefix_of_path(const char *path1, const char *path2);
! extern char *get_progname(const char *argv0);
  extern void get_share_path(const char *my_exec_path, char *ret_path);
  extern void get_etc_path(const char *my_exec_path, char *ret_path);
  extern void get_include_path(const char *my_exec_path, char *ret_path);
diff --git a/src/port/path.c b/src/port/path.c
new file mode 100644
index 72ca24c..ebfc94c
*** a/src/port/path.c
--- b/src/port/path.c
*************** path_is_prefix_of_path(const char *path1
*** 411,417 ****
   * Extracts the actual name of the program as called -
   * stripped of .exe suffix if any
   */
! const char *
  get_progname(const char *argv0)
  {
        const char *nodir_name;
--- 411,417 ----
   * Extracts the actual name of the program as called -
   * stripped of .exe suffix if any
   */
! char *
  get_progname(const char *argv0)
  {
        const char *nodir_name;

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Phil Sorber (#1)
Re: get_progname() should not be const char *?

Phil Sorber <phil@omniti.com> writes:

get_progname() returns a strdup()'d value. Shouldn't it then be simply
char * and not const char *? Otherwise free() complains loudly without
a cast.

I don't believe that callers should be trying to free() the result.
Whether it's been strdup'd or not is not any of their business.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#3Phil Sorber
phil@omniti.com
In reply to: Tom Lane (#2)
Re: get_progname() should not be const char *?

On Mon, Feb 4, 2013 at 10:52 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Phil Sorber <phil@omniti.com> writes:

get_progname() returns a strdup()'d value. Shouldn't it then be simply
char * and not const char *? Otherwise free() complains loudly without
a cast.

I don't believe that callers should be trying to free() the result.
Whether it's been strdup'd or not is not any of their business.

Is that just because of the nature of this specific function?

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#4Robert Haas
robertmhaas@gmail.com
In reply to: Phil Sorber (#3)
Re: get_progname() should not be const char *?

On Tue, Feb 5, 2013 at 12:18 AM, Phil Sorber <phil@omniti.com> wrote:

On Mon, Feb 4, 2013 at 10:52 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Phil Sorber <phil@omniti.com> writes:

get_progname() returns a strdup()'d value. Shouldn't it then be simply
char * and not const char *? Otherwise free() complains loudly without
a cast.

I don't believe that callers should be trying to free() the result.
Whether it's been strdup'd or not is not any of their business.

Is that just because of the nature of this specific function?

I can't presume to speak for Tom, but I think so. Sometimes the API
of a function includes the notion that the caller should pfree the
result. Sometimes it doesn't. The advantage of NOT including that in
the API contract is that you can sometimes do optimizations that would
be impossible otherwise - e.g. you can return the same palloc'd string
on successive calls to the function; or you can sometimes return a
statically allocated string.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#5Tom Lane
tgl@sss.pgh.pa.us
In reply to: Robert Haas (#4)
Re: get_progname() should not be const char *?

Robert Haas <robertmhaas@gmail.com> writes:

On Tue, Feb 5, 2013 at 12:18 AM, Phil Sorber <phil@omniti.com> wrote:

On Mon, Feb 4, 2013 at 10:52 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

I don't believe that callers should be trying to free() the result.
Whether it's been strdup'd or not is not any of their business.

Is that just because of the nature of this specific function?

I can't presume to speak for Tom, but I think so. Sometimes the API
of a function includes the notion that the caller should pfree the
result. Sometimes it doesn't. The advantage of NOT including that in
the API contract is that you can sometimes do optimizations that would
be impossible otherwise - e.g. you can return the same palloc'd string
on successive calls to the function; or you can sometimes return a
statically allocated string.

Yeah. In this particular case, it seems rather obvious that the
function should be returning the same string each time --- if it's
actually doing a fresh malloc, that sounds like a bug.

But in any case, adding or removing a const qualifier from a function's
result typically goes along with an API-contract change as to whether
the caller is supposed to free the result or not. My objection here
was specifically that I don't believe the contract for get_progname
includes caller-free now, and I don't want it to start being that.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#6Phil Sorber
phil@omniti.com
In reply to: Tom Lane (#5)
Re: get_progname() should not be const char *?

On Wed, Feb 6, 2013 at 11:22 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Robert Haas <robertmhaas@gmail.com> writes:

On Tue, Feb 5, 2013 at 12:18 AM, Phil Sorber <phil@omniti.com> wrote:

On Mon, Feb 4, 2013 at 10:52 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

I don't believe that callers should be trying to free() the result.
Whether it's been strdup'd or not is not any of their business.

Is that just because of the nature of this specific function?

I can't presume to speak for Tom, but I think so. Sometimes the API
of a function includes the notion that the caller should pfree the
result. Sometimes it doesn't. The advantage of NOT including that in
the API contract is that you can sometimes do optimizations that would
be impossible otherwise - e.g. you can return the same palloc'd string
on successive calls to the function; or you can sometimes return a
statically allocated string.

Yeah. In this particular case, it seems rather obvious that the
function should be returning the same string each time --- if it's
actually doing a fresh malloc, that sounds like a bug.

It does, but it's noted in a comment that it's only expected to be run once.

But in any case, adding or removing a const qualifier from a function's
result typically goes along with an API-contract change as to whether
the caller is supposed to free the result or not. My objection here
was specifically that I don't believe the contract for get_progname
includes caller-free now, and I don't want it to start being that.

That's fair. Thanks for the explanation.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers