You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Ivan Zhakov <ch...@gmail.com> on 2006/12/28 08:20:59 UTC
Hide internal data in svn_wc_entry_t struct
Some data in svn_wc_entry_t is local and valuable only for libsvn_wc.
One of this exampe is new field keep_local which I've to introduce due
implementing svn rm --keep-local.
And I don't see reasons to publish such data and maintain it using ABI
policy, revv and etc.
So my idea is introduce private structure to store such data:
In include/svn_wc.h:
typedef struct svn_wc_entry_private_t;
typedef struct svn_wc_entry_t
{
/* Public fields goes here. */
/* Private fields. */
svn_wc_entry_private_t private;
} svn_wc_entry_t;
And in libsvn_wc/wc.h we describe our private struct:
typedef struct svn_wc_entry_private_t;
{
svn_boolean_t keep_local;
} struct svn_wc_entry_private_t;
--
Ivan Zhakov
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
RE: Hide internal data in svn_wc_entry_t struct
Posted by Madan U S <ma...@collab.net>.
> -----Original Message-----
> From: Malcolm Rowe [mailto:malcolm-svn-dev@farside.org.uk]
> Sent: Thu 12/28/2006 3:11 PM
> To: Madan U S
> Cc: Ivan Zhakov; svn-dev
> Subject: Re: Hide internal data in svn_wc_entry_t struct
>
> On Thu, Dec 28, 2006 at 02:24:18PM +0530, Madan U S wrote:
> > > Do you mean use name "baton" or use baton syntax with "void *"?
> >
> > Both.
> >
> > void, because, introducing svn_wc_entry_private_t still leaves space for ABI revving, when we have svn_wc_entry_private2_t.
> >
> > the word 'baton' because this would mean wc specific baton containing wc specific information... and the term is pretty well-known in > svn-dev-land.
> >
>
> Batons are usually provided by the caller, not the library. They're
> handed back to a callback, hence the name.
Well said! But I have a question here. Digging a bit deeper, could the 'caller' be a library itself?
Regards,
Madan.
Re: Hide internal data in svn_wc_entry_t struct
Posted by Ivan Zhakov <ch...@gmail.com>.
On 12/29/06, Justin Erenkrantz <ju...@erenkrantz.com> wrote:
> On Thu, Dec 28, 2006 at 11:51:57PM -0800, Daniel Rall wrote:
> > I too prefer a pointer to an opaque type, since the pointer will
> > always point to the same type. Declare the typedef in a public
> > header, note it's private data, and define it in a private header.
> >
> > The couple lines the declaration takes doesn't cost us much in terms
> > of additional complexity for users of our header files, and ought to
> > leave us with a cleaner internal implementation.
>
> +1. -- justin
>
Ok, I'll do it after committing my rm --keep-local changes.
--
Ivan Zhakov
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Hide internal data in svn_wc_entry_t struct
Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
On Thu, Dec 28, 2006 at 11:51:57PM -0800, Daniel Rall wrote:
> I too prefer a pointer to an opaque type, since the pointer will
> always point to the same type. Declare the typedef in a public
> header, note it's private data, and define it in a private header.
>
> The couple lines the declaration takes doesn't cost us much in terms
> of additional complexity for users of our header files, and ought to
> leave us with a cleaner internal implementation.
+1. -- justin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Hide internal data in svn_wc_entry_t struct
Posted by Daniel Rall <dl...@collab.net>.
On Thu, 28 Dec 2006, Ivan Zhakov wrote:
> On 12/28/06, Malcolm Rowe <ma...@farside.org.uk> wrote:
> >On Thu, Dec 28, 2006 at 02:24:18PM +0530, Madan U S wrote:
> >> > Do you mean use name "baton" or use baton syntax with "void *"?
> >>
> >> Both.
> >>
> >> void, because, introducing svn_wc_entry_private_t still leaves space for
> >> ABI revving, when we have svn_wc_entry_private2_t.
> >>
> >> the word 'baton' because this would mean wc specific baton containing wc
> >specific information... and the term is pretty well-known in svn-dev-land.
> >
> >Batons are usually provided by the caller, not the library. They're
> >handed back to a callback, hence the name. An existing example of
> >private data would be within the FS layer, within an svn_fs_t, where we
> >just call it 'fsap_data'.
> >
> >From the caller's point of view, I don't believe there's any difference
> >between a 'void *' and a pointer-to-opaque type, since all pointers are
> >the same size (and you'll never be able to dereference either); we'd
> >therefore never need to create a svn_wc_entry_private2_t.
>
> Oh, sorry. Of course we should use pointer to svn_wc_entry_private_t.
>
> >That said, we don't need the type publically named for any other reason,
> >so just creating a 'void *private; /* private data for libsvn_wc */'
> >field would make sense to me.
>
> I prefer use pointer to published structure because we can use it with
> type cast. Just write: entry->private->keep_local = TRUE;
>
> If we'll use void *, we have to cast it like this:
> ((svn_wc_entry_private*)entry->private)->keep_local = TRUE;
> Which is ugly for me.
I too prefer a pointer to an opaque type, since the pointer will
always point to the same type. Declare the typedef in a public
header, note it's private data, and define it in a private header.
The couple lines the declaration takes doesn't cost us much in terms
of additional complexity for users of our header files, and ought to
leave us with a cleaner internal implementation.
Re: Hide internal data in svn_wc_entry_t struct
Posted by Ivan Zhakov <ch...@gmail.com>.
On 12/28/06, Malcolm Rowe <ma...@farside.org.uk> wrote:
> On Thu, Dec 28, 2006 at 02:24:18PM +0530, Madan U S wrote:
> > > Do you mean use name "baton" or use baton syntax with "void *"?
> >
> > Both.
> >
> > void, because, introducing svn_wc_entry_private_t still leaves space for ABI revving, when we have svn_wc_entry_private2_t.
> >
> > the word 'baton' because this would mean wc specific baton containing wc specific information... and the term is pretty well-known in svn-dev-land.
> >
>
> Batons are usually provided by the caller, not the library. They're
> handed back to a callback, hence the name. An existing example of
> private data would be within the FS layer, within an svn_fs_t, where we
> just call it 'fsap_data'.
>
> From the caller's point of view, I don't believe there's any difference
> between a 'void *' and a pointer-to-opaque type, since all pointers are
> the same size (and you'll never be able to dereference either); we'd
> therefore never need to create a svn_wc_entry_private2_t.
Oh, sorry. Of course we should use pointer to svn_wc_entry_private_t.
>
> That said, we don't need the type publically named for any other reason,
> so just creating a 'void *private; /* private data for libsvn_wc */'
> field would make sense to me.
>
I prefer use pointer to published structure because we can use it with
type cast. Just write: entry->private->keep_local = TRUE;
If we'll use void *, we have to cast it like this:
((svn_wc_entry_private*)entry->private)->keep_local = TRUE;
Which is ugly for me.
--
Ivan Zhakov
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Hide internal data in svn_wc_entry_t struct
Posted by Malcolm Rowe <ma...@farside.org.uk>.
On Thu, Dec 28, 2006 at 02:24:18PM +0530, Madan U S wrote:
> > Do you mean use name "baton" or use baton syntax with "void *"?
>
> Both.
>
> void, because, introducing svn_wc_entry_private_t still leaves space for ABI revving, when we have svn_wc_entry_private2_t.
>
> the word 'baton' because this would mean wc specific baton containing wc specific information... and the term is pretty well-known in svn-dev-land.
>
Batons are usually provided by the caller, not the library. They're
handed back to a callback, hence the name. An existing example of
private data would be within the FS layer, within an svn_fs_t, where we
just call it 'fsap_data'.
From the caller's point of view, I don't believe there's any difference
between a 'void *' and a pointer-to-opaque type, since all pointers are
the same size (and you'll never be able to dereference either); we'd
therefore never need to create a svn_wc_entry_private2_t.
That said, we don't need the type publically named for any other reason,
so just creating a 'void *private; /* private data for libsvn_wc */'
field would make sense to me.
Regards,
Malcolm
RE: Hide internal data in svn_wc_entry_t struct
Posted by Madan U S <ma...@collab.net>.
Sorry about top-posting.
> Do you mean use name "baton" or use baton syntax with "void *"?
Both.
void, because, introducing svn_wc_entry_private_t still leaves space for ABI revving, when we have svn_wc_entry_private2_t.
the word 'baton' because this would mean wc specific baton containing wc specific information... and the term is pretty well-known in svn-dev-land.
Regards,
Madan.
-----Original Message-----
From: Ivan Zhakov [mailto:chemodax@gmail.com]
Sent: Thu 12/28/2006 2:11 PM
To: Madan U S
Cc: svn-dev
Subject: Re: Hide internal data in svn_wc_entry_t struct
On 12/28/06, Madan U S <ma...@collab.net> wrote:
>
>
>
> > -----Original Message-----
> > From: Ivan Zhakov [mailto:chemodax@gmail.com]
> > Sent: Thu 12/28/2006 1:50 PM
> > To: svn-dev
> > Subject: Hide internal data in svn_wc_entry_t struct
> >
> > Some data in svn_wc_entry_t is local and valuable only for libsvn_wc.
> > One of this exampe is new field keep_local which I've to introduce due
> > implementing svn rm --keep-local.
> > And I don't see reasons to publish such data and maintain it using ABI
> > policy, revv and etc.
> > So my idea is introduce private structure to store such data:
> > In include/svn_wc.h:
> > typedef struct svn_wc_entry_private_t;
>
> Sounds like a good candidate for a baton....
>
Do you mean use name "baton" or use baton syntax with "void *"?
--
Ivan Zhakov
Re: Hide internal data in svn_wc_entry_t struct
Posted by Ivan Zhakov <ch...@gmail.com>.
On 12/28/06, Madan U S <ma...@collab.net> wrote:
>
>
>
> > -----Original Message-----
> > From: Ivan Zhakov [mailto:chemodax@gmail.com]
> > Sent: Thu 12/28/2006 1:50 PM
> > To: svn-dev
> > Subject: Hide internal data in svn_wc_entry_t struct
> >
> > Some data in svn_wc_entry_t is local and valuable only for libsvn_wc.
> > One of this exampe is new field keep_local which I've to introduce due
> > implementing svn rm --keep-local.
> > And I don't see reasons to publish such data and maintain it using ABI
> > policy, revv and etc.
> > So my idea is introduce private structure to store such data:
> > In include/svn_wc.h:
> > typedef struct svn_wc_entry_private_t;
>
> Sounds like a good candidate for a baton....
>
Do you mean use name "baton" or use baton syntax with "void *"?
--
Ivan Zhakov
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
RE: Hide internal data in svn_wc_entry_t struct
Posted by Madan U S <ma...@collab.net>.
> -----Original Message-----
> From: Ivan Zhakov [mailto:chemodax@gmail.com]
> Sent: Thu 12/28/2006 1:50 PM
> To: svn-dev
> Subject: Hide internal data in svn_wc_entry_t struct
>
> Some data in svn_wc_entry_t is local and valuable only for libsvn_wc.
> One of this exampe is new field keep_local which I've to introduce due
> implementing svn rm --keep-local.
> And I don't see reasons to publish such data and maintain it using ABI
> policy, revv and etc.
> So my idea is introduce private structure to store such data:
> In include/svn_wc.h:
> typedef struct svn_wc_entry_private_t;
Sounds like a good candidate for a baton....
Regards,
Madan.