You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Karl Fogel <kf...@red-bean.com> on 2007/09/12 00:33:27 UTC
Re: svn commit: r26533 - in trunk/subversion: include libsvn_client libsvn_subr libsvn_wc
dionisos@tigris.org writes:
> Log:
> Create iterator drivers
> (with some code cleanup and pool use fixes to show their value).
Yay!
> --- (empty file)
> +++ trunk/subversion/include/svn_iter.h Mon Sep 10 23:24:39 2007
> @@ -0,0 +1,124 @@
>
> [...]
>
> +/** Iterate over the elements in @a hash, calling @a func for each one until
> + * there are no more elements or @a func returns an error.
> + *
> + * Uses @a pool for temporary allocations.
> + *
> + * On return - if @a func returns no errors - @a completed will be set
> + * to @c TRUE.
> + *
> + * If @a func returns an error other than @c SVN_ERR_ITER_BREAK, that
> + * error is returned. When @a func returns @c SVN_ERR_ITER_BREAK,
> + * iteration is interrupted, but no error is returned and @a completed is
> + * set to @c FALSE.
> + *
> + * @since New in 1.5.
> + */
> +svn_error_t *
> +svn_iter_apr_hash(svn_boolean_t *completed,
> + apr_hash_t *hash,
> + svn_iter_apr_hash_cb_t func,
> + void *baton,
> + apr_pool_t *pool);
Should be "*completed" in doc string (same for svn_iter_apr_array).
Both fixed in r26542, np.
> +#define SVN_ITER_BREAK SVN_NO_ERROR + 1
What's this for?
> +/** Helper macro to break looping in svn_iter_apr_array() and
> + * svn_iter_apr_hash() driven loops.
> + *
> + * @note The error is just a means of communicating between
> + * driver and callback. There is no need for it to exist
> + * past the lifetime of the iterpool.
> + *
> + * @since New in 1.5.
> + */
> +#define svn_iter_break(pool) \
> + do { \
> + svn_error_t *tmp__err = apr_pcalloc((pool), sizeof(*tmp__err)); \
> + tmp__err->apr_err = SVN_ERR_ITER_BREAK; \
> + return tmp__err; \
> + } while (0)
Hmmm, an idea: could we just define a static error object here?
svn_error_t svn_iter_shared_break = {
SVN_ERR_ITER_BREAK, /* apr_status_t apr_err */
NULL, /* const char *message */
NULL, /* struct svn_error_t *child */
NULL, /* apr_pool_t *pool */
__FILE__, /* const char *file; */
__LINE__ /* long line; */
}
Then svn_iter.c could just use '&svn_iter_shared_break', and we'd
avoid some overhead.
-Karl
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r26533 - in trunk/subversion: include libsvn_client libsvn_subr libsvn_wc
Posted by Daniel Rall <dl...@finemaltcoding.com>.
On Sep 12, 2007, at 2:00 AM, Erik Huelsmann wrote:
> On 9/12/07, Ivan Zhakov <ch...@gmail.com> wrote:
...
>> I see two ways to fix it:
>> - Create new function to return svn_iter_break error object.
>> Something
>> like svn_err_iter_break()
>
>> - Define svn_iter_shared_break variable in header file with static
>> modifier. In this case variable will be copied in each file using
>> this
>> header.
>>
>> Personally I don't like casting 1 to pointer. I find it too hacky :)
>
> In that case, the first solution will do. I'll implement that.
I favor this solution, too.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r26533 - in trunk/subversion: include libsvn_client libsvn_subr libsvn_wc
Posted by Erik Huelsmann <eh...@gmail.com>.
On 9/12/07, Ivan Zhakov <ch...@gmail.com> wrote:
> /On 9/12/07, Erik Huelsmann <eh...@gmail.com> wrote:
> > On 9/12/07, Karl Fogel <kf...@red-bean.com> wrote:
> > > "Erik Huelsmann" <eh...@gmail.com> writes:
> > > > I'll need to look at how we solved this in svn_ctypes.h, but as far as
> > > > I recall there were problems with exporting variables from DLLs on
> > > > Windows, because supposedly MSVC needs to reference a pointer to the
> > > > variable whereas other compilers support manipulating the variable
> > > > directly, see http://svn.haxx.se/dev/archive-2005-12/0364.shtml
> > >
> > > Hmm. I didn't quite understand that, and unfortunately svn.haxx.se is
> > > unreachable from my home network right now. We can't just define the
> > > object once in svn_iter.c, and then declare it "extern" in svn_iter.h?
> > > Does MSVC have a problem with that? (I thought it was standard C89.)
> >
> > Oh, sure you can declare it as such, but it will only work with static
> > libs, not with DLLs. For DLLs you need to declare it with the MSVC
> > specific qualifier __cdecl(dllimport)/ __cdecl(dllexport). The
> > discussion I referenced concludes that the way ctyes was set up until
> > then was untennable. The end result is that ctypes was made internal
> > to libsvn_subr. (Although it looks like the header wasn't updated to
> > reflect that...)
> I see two ways to fix it:
> - Create new function to return svn_iter_break error object. Something
> like svn_err_iter_break()
> - Define svn_iter_shared_break variable in header file with static
> modifier. In this case variable will be copied in each file using this
> header.
>
> Personally I don't like casting 1 to pointer. I find it too hacky :)
In that case, the first solution will do. I'll implement that.
Thanks for the feedback,
Erik.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r26533 - in trunk/subversion: include libsvn_client libsvn_subr libsvn_wc
Posted by Ivan Zhakov <ch...@gmail.com>.
/On 9/12/07, Erik Huelsmann <eh...@gmail.com> wrote:
> On 9/12/07, Karl Fogel <kf...@red-bean.com> wrote:
> > "Erik Huelsmann" <eh...@gmail.com> writes:
> > > I'll need to look at how we solved this in svn_ctypes.h, but as far as
> > > I recall there were problems with exporting variables from DLLs on
> > > Windows, because supposedly MSVC needs to reference a pointer to the
> > > variable whereas other compilers support manipulating the variable
> > > directly, see http://svn.haxx.se/dev/archive-2005-12/0364.shtml
> >
> > Hmm. I didn't quite understand that, and unfortunately svn.haxx.se is
> > unreachable from my home network right now. We can't just define the
> > object once in svn_iter.c, and then declare it "extern" in svn_iter.h?
> > Does MSVC have a problem with that? (I thought it was standard C89.)
>
> Oh, sure you can declare it as such, but it will only work with static
> libs, not with DLLs. For DLLs you need to declare it with the MSVC
> specific qualifier __cdecl(dllimport)/ __cdecl(dllexport). The
> discussion I referenced concludes that the way ctyes was set up until
> then was untennable. The end result is that ctypes was made internal
> to libsvn_subr. (Although it looks like the header wasn't updated to
> reflect that...)
I see two ways to fix it:
- Create new function to return svn_iter_break error object. Something
like svn_err_iter_break()
- Define svn_iter_shared_break variable in header file with static
modifier. In this case variable will be copied in each file using this
header.
Personally I don't like casting 1 to pointer. I find it too hacky :)
--
Ivan Zhakov
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r26533 - in trunk/subversion: include libsvn_client libsvn_subr libsvn_wc
Posted by Daniel Rall <dl...@finemaltcoding.com>.
On Sep 12, 2007, at 2:13 AM, Karl Fogel wrote:
> "Ivan Zhakov" <ch...@gmail.com> writes:
>> I'd like idea to test for err->pool in svn_error_clear(), but we
>> cannot. err->pool is used for functions like svn_handle_error2().
>
> Ah, in other words: the problem isn't that we can't test for it, but
> rather that we simply can't let it be NULL (for most error objects,
> though not in the svn_iter_break case).
Yup. I've experienced the case Ivan points out when neglecting to
return properly initialized svn_error_t's, and they've been
propagated through the call stack to the command-line client's error
handling.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r26533 - in trunk/subversion: include libsvn_client libsvn_subr libsvn_wc
Posted by Ivan Zhakov <ch...@gmail.com>.
On 9/12/07, Karl Fogel <kf...@red-bean.com> wrote:
> "Ivan Zhakov" <ch...@gmail.com> writes:
> > I'd like idea to test for err->pool in svn_error_clear(), but we
> > cannot. err->pool is used for functions like svn_handle_error2().
>
> Ah, in other words: the problem isn't that we can't test for it, but
> rather that we simply can't let it be NULL (for most error objects,
> though not in the svn_iter_break case).
>
Yep, you are right.
--
Ivan Zhakov
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r26533 - in trunk/subversion: include libsvn_client libsvn_subr libsvn_wc
Posted by Karl Fogel <kf...@red-bean.com>.
"Ivan Zhakov" <ch...@gmail.com> writes:
> I'd like idea to test for err->pool in svn_error_clear(), but we
> cannot. err->pool is used for functions like svn_handle_error2().
Ah, in other words: the problem isn't that we can't test for it, but
rather that we simply can't let it be NULL (for most error objects,
though not in the svn_iter_break case).
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r26533 - in trunk/subversion: include libsvn_client libsvn_subr libsvn_wc
Posted by Ivan Zhakov <ch...@gmail.com>.
On 9/12/07, Karl Fogel <kf...@red-bean.com> wrote:
> "Erik Huelsmann" <eh...@gmail.com> writes:
> > But only slightly: neither one nor the other can be passed to
> > svn_error_clear(); both will cause a SEGFAULT, the former because it's
> > an invalid pointer, the latter because it doesn't have a valid POOL
> > object in the pool field....
>
> But that's okay, we don't ever need to pass it to svn_error_clear().
> In fact, svn_iter_apr_hash() and svn_iter_apr_array() already test
>
> if (err->pool)
>
> before clearing. Which I guess you know, since you wrote it :-).
>
> (And svn_error_clear() could test for err->pool itself, which might
> not be such a bad idea...)
>
I'd like idea to test for err->pool in svn_error_clear(), but we
cannot. err->pool is used for functions like svn_handle_error2().
--
Ivan Zhakov
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r26533 - in trunk/subversion: include libsvn_client libsvn_subr libsvn_wc
Posted by Karl Fogel <kf...@red-bean.com>.
"Erik Huelsmann" <eh...@gmail.com> writes:
> But only slightly: neither one nor the other can be passed to
> svn_error_clear(); both will cause a SEGFAULT, the former because it's
> an invalid pointer, the latter because it doesn't have a valid POOL
> object in the pool field....
But that's okay, we don't ever need to pass it to svn_error_clear().
In fact, svn_iter_apr_hash() and svn_iter_apr_array() already test
if (err->pool)
before clearing. Which I guess you know, since you wrote it :-).
(And svn_error_clear() could test for err->pool itself, which might
not be such a bad idea...)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r26533 - in trunk/subversion: include libsvn_client libsvn_subr libsvn_wc
Posted by Erik Huelsmann <eh...@gmail.com>.
On 9/12/07, Karl Fogel <kf...@red-bean.com> wrote:
> "Erik Huelsmann" <eh...@gmail.com> writes:
> > I'll need to look at how we solved this in svn_ctypes.h, but as far as
> > I recall there were problems with exporting variables from DLLs on
> > Windows, because supposedly MSVC needs to reference a pointer to the
> > variable whereas other compilers support manipulating the variable
> > directly, see http://svn.haxx.se/dev/archive-2005-12/0364.shtml
>
> Hmm. I didn't quite understand that, and unfortunately svn.haxx.se is
> unreachable from my home network right now. We can't just define the
> object once in svn_iter.c, and then declare it "extern" in svn_iter.h?
> Does MSVC have a problem with that? (I thought it was standard C89.)
Oh, sure you can declare it as such, but it will only work with static
libs, not with DLLs. For DLLs you need to declare it with the MSVC
specific qualifier __cdecl(dllimport)/ __cdecl(dllexport). The
discussion I referenced concludes that the way ctyes was set up until
then was untennable. The end result is that ctypes was made internal
to libsvn_subr. (Although it looks like the header wasn't updated to
reflect that...)
> > I guess returning "(svn_error_t *)1" is the most portable solution to
> > returning a constant-and-known-pointer value to check against...
> >
> > Comments?
>
> Either one is fine, but having it be a real error object would be
> better, of course.
But only slightly: neither one nor the other can be passed to
svn_error_clear(); both will cause a SEGFAULT, the former because it's
an invalid pointer, the latter because it doesn't have a valid POOL
object in the pool field....
bye,
Erik.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r26533 - in trunk/subversion: include libsvn_client libsvn_subr libsvn_wc
Posted by Karl Fogel <kf...@red-bean.com>.
"Erik Huelsmann" <eh...@gmail.com> writes:
> I'll need to look at how we solved this in svn_ctypes.h, but as far as
> I recall there were problems with exporting variables from DLLs on
> Windows, because supposedly MSVC needs to reference a pointer to the
> variable whereas other compilers support manipulating the variable
> directly, see http://svn.haxx.se/dev/archive-2005-12/0364.shtml
Hmm. I didn't quite understand that, and unfortunately svn.haxx.se is
unreachable from my home network right now. We can't just define the
object once in svn_iter.c, and then declare it "extern" in svn_iter.h?
Does MSVC have a problem with that? (I thought it was standard C89.)
> I guess returning "(svn_error_t *)1" is the most portable solution to
> returning a constant-and-known-pointer value to check against...
>
> Comments?
Either one is fine, but having it be a real error object would be
better, of course.
-Karl
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r26533 - in trunk/subversion: include libsvn_client libsvn_subr libsvn_wc
Posted by Erik Huelsmann <eh...@gmail.com>.
On 9/12/07, Karl Fogel <kf...@red-bean.com> wrote:
> dionisos@tigris.org writes:
> > Log:
> > Create iterator drivers
> > (with some code cleanup and pool use fixes to show their value).
>
> Yay!
>
> > --- (empty file)
> > +++ trunk/subversion/include/svn_iter.h Mon Sep 10 23:24:39 2007
> > @@ -0,0 +1,124 @@
> >
> > [...]
> >
> > +/** Iterate over the elements in @a hash, calling @a func for each one until
> > + * there are no more elements or @a func returns an error.
> > + *
> > + * Uses @a pool for temporary allocations.
> > + *
> > + * On return - if @a func returns no errors - @a completed will be set
> > + * to @c TRUE.
> > + *
> > + * If @a func returns an error other than @c SVN_ERR_ITER_BREAK, that
> > + * error is returned. When @a func returns @c SVN_ERR_ITER_BREAK,
> > + * iteration is interrupted, but no error is returned and @a completed is
> > + * set to @c FALSE.
> > + *
> > + * @since New in 1.5.
> > + */
> > +svn_error_t *
> > +svn_iter_apr_hash(svn_boolean_t *completed,
> > + apr_hash_t *hash,
> > + svn_iter_apr_hash_cb_t func,
> > + void *baton,
> > + apr_pool_t *pool);
>
> Should be "*completed" in doc string (same for svn_iter_apr_array).
> Both fixed in r26542, np.
>
> > +#define SVN_ITER_BREAK SVN_NO_ERROR + 1
>
> What's this for?
>
> > +/** Helper macro to break looping in svn_iter_apr_array() and
> > + * svn_iter_apr_hash() driven loops.
> > + *
> > + * @note The error is just a means of communicating between
> > + * driver and callback. There is no need for it to exist
> > + * past the lifetime of the iterpool.
> > + *
> > + * @since New in 1.5.
> > + */
> > +#define svn_iter_break(pool) \
> > + do { \
> > + svn_error_t *tmp__err = apr_pcalloc((pool), sizeof(*tmp__err)); \
> > + tmp__err->apr_err = SVN_ERR_ITER_BREAK; \
> > + return tmp__err; \
> > + } while (0)
>
> Hmmm, an idea: could we just define a static error object here?
>
> svn_error_t svn_iter_shared_break = {
> SVN_ERR_ITER_BREAK, /* apr_status_t apr_err */
> NULL, /* const char *message */
> NULL, /* struct svn_error_t *child */
> NULL, /* apr_pool_t *pool */
> __FILE__, /* const char *file; */
> __LINE__ /* long line; */
> }
Yes, we could. Actually, that's what the SVN_ITER_BREAK is about; it's
a remnant of a discussion with dlr in IRC where I said something along
the lines of 'having callbacks return "(svn_error_t *)1" will
eliminate the need to create a real error object'. However we both
didn't feel to good about returning a non-NULL pointer without it
being a pointer to a real object.
> Then svn_iter.c could just use '&svn_iter_shared_break', and we'd
> avoid some overhead.
I'll need to look at how we solved this in svn_ctypes.h, but as far as
I recall there were problems with exporting variables from DLLs on
Windows, because supposedly MSVC needs to reference a pointer to the
variable whereas other compilers support manipulating the variable
directly, see http://svn.haxx.se/dev/archive-2005-12/0364.shtml
I guess returning "(svn_error_t *)1" is the most portable solution to
returning a constant-and-known-pointer value to check against...
Comments?
bye,
Erik.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org