Thread: get_progname() should not be const char *?
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(constchar *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(constchar *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);
externvoid 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;
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
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
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
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
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