Thread: Re: [COMMITTERS] pgsql: Unify some tar functionality across different parts
Magnus Hagander <magnus@hagander.net> writes:
>> On Wed, Jan 2, 2013 at 4:14 AM, Andrew Dunstan <andrew@dunslane.net> wrote:
>>> This seems to have broken plperl builds on Windows.
> I'm not really sure what the best thing is to do here. We can either
> use the fact that we know that uid_t and gid_t are "int" on all our
> platforms, more or less, and change the tar api to use uid_t. Or put
> the tar functions in their own header and make sure we don't include
> that one anywhere that we include the perl headers.
Why are these very tar-specific functions being declared in such a
globally visible spot as port.h? That seems like a bad idea on its
face. IMO stuff in port.h ought to be about as globally applicable
as, say, malloc().
If there's actually a reason for that sort of namespace abuse, then
change their parameters to int --- IIRC, the tar format can't support
UIDs wider than 32 bits anyway.
regards, tom lane
Re: [COMMITTERS] pgsql: Unify some tar functionality across different parts
From
Magnus Hagander
Date:
On Wed, Jan 2, 2013 at 5:36 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Magnus Hagander <magnus@hagander.net> writes: >>> On Wed, Jan 2, 2013 at 4:14 AM, Andrew Dunstan <andrew@dunslane.net> wrote: >>>> This seems to have broken plperl builds on Windows. > >> I'm not really sure what the best thing is to do here. We can either >> use the fact that we know that uid_t and gid_t are "int" on all our >> platforms, more or less, and change the tar api to use uid_t. Or put >> the tar functions in their own header and make sure we don't include >> that one anywhere that we include the perl headers. > > Why are these very tar-specific functions being declared in such a > globally visible spot as port.h? That seems like a bad idea on its > face. IMO stuff in port.h ought to be about as globally applicable > as, say, malloc(). It's where we put most of the things from src/port in. I take it you suggest moving it to a special say include/pgtar.h file? > If there's actually a reason for that sort of namespace abuse, then > change their parameters to int --- IIRC, the tar format can't support > UIDs wider than 32 bits anyway. Andrew suggested #ifndef'ing the declarations the same way that the typedefs for uid_t and gid_t are done as another optin. I don't really find either one of them non-kludgy :) -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/
Magnus Hagander <magnus@hagander.net> writes:
> On Wed, Jan 2, 2013 at 5:36 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Why are these very tar-specific functions being declared in such a
>> globally visible spot as port.h? That seems like a bad idea on its
>> face. IMO stuff in port.h ought to be about as globally applicable
>> as, say, malloc().
> It's where we put most of the things from src/port in.
Might be time to revisit that, or perhaps better reconsider what we're
putting in src/port/. The idea that anything that we want in both
frontend and backend is a porting issue seems a bit busted in itself.
> I take it you suggest moving it to a special say include/pgtar.h file?
Works for me.
regards, tom lane
Re: [COMMITTERS] pgsql: Unify some tar functionality across different parts
From
Magnus Hagander
Date:
On Wed, Jan 2, 2013 at 5:44 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Magnus Hagander <magnus@hagander.net> writes: >> On Wed, Jan 2, 2013 at 5:36 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >>> Why are these very tar-specific functions being declared in such a >>> globally visible spot as port.h? That seems like a bad idea on its >>> face. IMO stuff in port.h ought to be about as globally applicable >>> as, say, malloc(). > >> It's where we put most of the things from src/port in. > > Might be time to revisit that, or perhaps better reconsider what we're > putting in src/port/. The idea that anything that we want in both > frontend and backend is a porting issue seems a bit busted in itself. Yeah, true - but there's certainly other parts that think the same, so that's a separate issue. >> I take it you suggest moving it to a special say include/pgtar.h file? > > Works for me. Applied. Should hopefully work once Alvaro gets the buildfarm back to green. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/