You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Madan U Sreenivasan <ma...@collab.net> on 2005/08/22 17:17:15 UTC
[PATCH] Issue 443 : post-commit hook (error) output lost: Step 3
Pl. find attached another small step towards issue #443's solution...
Pl. note that subversion/libsvn_subr/svn_helpers.c is a new file...
Regards,
Madan.
Re: [PATCH] Issue 443 : post-commit hook (error) output lost: Step
3
Posted by Madan U Sreenivasan <ma...@collab.net>.
On Mon, 2005-08-22 at 22:47, Madan U Sreenivasan wrote:
> Pl. find attached another small step towards issue #443's solution...
Just a humble reminder note for committers to notice this patch!
>
> Pl. note that subversion/libsvn_subr/svn_helpers.c is a new file...
>
> Regards,
> Madan.
>
> ______________________________________________________________________
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Issue 443 : post-commit hook (error) output lost: Step
3
Posted by Madan U Sreenivasan <ma...@collab.net>.
On Thu, 2005-08-25 at 08:13, kfogel@collab.net wrote:
> Madan U Sreenivasan <ma...@collab.net> writes:
> > Pl. find attached another small step towards issue #443's solution...
> [snip]
> That way if we have to rev the type later, to say svn_commit_info2_t,
> there's only one line to change -- and no possibility of forgetting to
> fix the allocation and thereby getting seg faults :-).
>
> The name "svn_helpers.c" is a bit generic, and the "svn" prefix is
> redundant. How would you feel about "types.c" or "constructors.c"
> instead? That would be somewhat descriptive, while being generic
> enough to be useful for other constructors later.
Pl. find patch and log message attached. Also corrected the grammar
mistake.
HTH.
Regards,
Madan.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Issue 443 : post-commit hook (error) output lost: Step 3
Posted by kf...@collab.net.
kfogel@collab.net writes:
> We usually allocate based on the variable itself, like this:
>
> sizeof (*commit_info)
>
> That way if we have to rev the type later, to say svn_commit_info2_t,
> there's only one line to change -- and no possibility of forgetting to
> fix the allocation and thereby getting seg faults :-).
>
> The name "svn_helpers.c" is a bit generic, and the "svn" prefix is
> redundant. How would you feel about "types.c" or "constructors.c"
> instead? That would be somewhat descriptive, while being generic
> enough to be useful for other constructors later.
>
> Thoughts?
I meant to say this in my earlier message, and then forgot:
No need to resubmit the patch. I can take care of the above changes
when committing. I just wanted your opinion on the new file's name
before I did anything.
-Karl
--
www.collab.net <> CollabNet | Distributed Development On Demand
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Issue 443 : post-commit hook (error) output lost: Step
3
Posted by Madan U Sreenivasan <ma...@collab.net>.
On Sat, 2005-08-27 at 01:27, kfogel@collab.net wrote:
> "Peter N. Lundblad" <pe...@famlundblad.se> writes:
> > On Thu, 25 Aug 2005, Madan U Sreenivasan wrote:
> > > oops, sorry, didnt attach the patch and log.... here comes....
> > >
> > r15930.
>
> Thanks Peter, you beat me to it by about ten minutes!
:) Thank you both.... Peter, as next step am gonna remove all usage of
svn_client_commit_info2_t and use svn_commit_info_t.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Issue 443 : post-commit hook (error) output lost: Step 3
Posted by kf...@collab.net.
"Peter N. Lundblad" <pe...@famlundblad.se> writes:
> On Thu, 25 Aug 2005, Madan U Sreenivasan wrote:
> > oops, sorry, didnt attach the patch and log.... here comes....
> >
> r15930.
Thanks Peter, you beat me to it by about ten minutes!
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Issue 443 : post-commit hook (error) output lost: Step
3
Posted by "Peter N. Lundblad" <pe...@famlundblad.se>.
On Thu, 25 Aug 2005, Madan U Sreenivasan wrote:
> oops, sorry, didnt attach the patch and log.... here comes....
>
r15930.
Thanks,
//Peter
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Issue 443 : post-commit hook (error) output lost: Step
3
Posted by Madan U Sreenivasan <ma...@collab.net>.
oops, sorry, didnt attach the patch and log.... here comes....
On Thu, 2005-08-25 at 13:04, Madan U Sreenivasan wrote:
> On Thu, 2005-08-25 at 08:13, kfogel@collab.net wrote:
> > Madan U Sreenivasan <ma...@collab.net> writes:
> > > Pl. find attached another small step towards issue #443's solution...
> > [snip]
> > We usually allocate based on the variable itself, like this:
> >
> > sizeof (*commit_info)
> >
> > That way if we have to rev the type later, to say svn_commit_info2_t,
> > there's only one line to change -- and no possibility of forgetting to
> > fix the allocation and thereby getting seg faults :-).
> Cool. Good thing. I'll stick to this too.
> >
> > The name "svn_helpers.c" is a bit generic, and the "svn" prefix is
> > redundant. How would you feel about "types.c" or "constructors.c"
> > instead? That would be somewhat descriptive, while being generic
> > enough to be useful for other constructors later.
> constructors.c would be great. very specific.
>
> am sending a patch too, might be helpful if you are not already finished
> tweaking yourself.
> Thank you, Karl.
> Regards,
> Madan.
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Issue 443 : post-commit hook (error) output lost: Step
3
Posted by Madan U Sreenivasan <ma...@collab.net>.
On Thu, 2005-08-25 at 08:13, kfogel@collab.net wrote:
> Madan U Sreenivasan <ma...@collab.net> writes:
> > Pl. find attached another small step towards issue #443's solution...
> [snip]
> We usually allocate based on the variable itself, like this:
>
> sizeof (*commit_info)
>
> That way if we have to rev the type later, to say svn_commit_info2_t,
> there's only one line to change -- and no possibility of forgetting to
> fix the allocation and thereby getting seg faults :-).
Cool. Good thing. I'll stick to this too.
>
> The name "svn_helpers.c" is a bit generic, and the "svn" prefix is
> redundant. How would you feel about "types.c" or "constructors.c"
> instead? That would be somewhat descriptive, while being generic
> enough to be useful for other constructors later.
constructors.c would be great. very specific.
am sending a patch too, might be helpful if you are not already finished
tweaking yourself.
Thank you, Karl.
Regards,
Madan.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Issue 443 : post-commit hook (error) output lost: Step 3
Posted by kf...@collab.net.
Madan U Sreenivasan <ma...@collab.net> writes:
> Pl. find attached another small step towards issue #443's solution...
>
> Pl. note that subversion/libsvn_subr/svn_helpers.c is a new file...
>
> Regards,
> Madan.
>
> Partial fix for Issue #443: post-commit hook script (error) output lost
> This is step 3 : Introduce new type svn_commit_info_t that can hold
> all data required about a commit. Create a constructor too.
>
> * subversion/include/svn_types.h
> (svn_commit_info_t): New type.
> (svn_create_commit_info): New function. Constructor for
> svn_commit_info_t.
>
> * subversion/libsvn_subr/svn_helpers.c: New file.
> (svn_create_commit_info): New helper function. Constructor for
> svn_commit_info_t.
Nice log message!
> Index: subversion/include/svn_types.h
> ===================================================================
> --- subversion/include/svn_types.h (revision 15883)
> +++ subversion/include/svn_types.h (working copy)
> @@ -297,6 +297,48 @@
> /** @} */
>
>
> +/** All information about commits.
> + *
> + * @note Objects of this type should always be created using the
> + * svn_create_commit_info() function.
> + *
> + * @since New in 1.3.
> + */
> +typedef struct svn_commit_info_t
> +{
> + /** just-committed revision. */
> + svn_revnum_t revision;
> +
> + /** server-side date of the commit. */
> + const char *date;
> +
> + /** author of the commit. */
> + const char *author;
> +
> + /** error message from post-commit hook, or NULL. */
> + const char *post_commit_err;
> +
> +} svn_commit_info_t;
Beautifully documented, and thank you for including the reminder to
always use the constructor function.
Grammar note: say "All information about a commit."
> +
> +/**
> + * Allocate an object of type @c svn_commit_info_t in @a pool and
> + * return it.
> + *
> + * The @c revision field of the new struct is set to @c
> + * SVN_INVALID_REVNUM. All other fields are initialized to @c NULL.
> + *
> + * @note Any object of the type @c svn_commit_info_t should
> + * be created using this function.
> + * This is to provide for extending the svn_commit_info_t in
> + * the future.
> + *
> + * @since New in 1.3.
> + */
> +svn_commit_info_t *
> +svn_create_commit_info (apr_pool_t *pool);
> +
> +
> /** A structure to represent a path that changed for a log entry. */
> typedef struct svn_log_changed_path_t
> {
> Index: subversion/libsvn_subr/svn_helpers.c
> ===================================================================
> --- subversion/libsvn_subr/svn_helpers.c (revision 0)
> +++ subversion/libsvn_subr/svn_helpers.c (revision 0)
> @@ -0,0 +1,34 @@
> +/*
> + * svn_helpers.c : Helper functions like constructors, ...
> + *
> + * ====================================================================
> + * Copyright (c) 2005 CollabNet. All rights reserved.
> + *
> + * This software is licensed as described in the file COPYING, which
> + * you should have received as part of this distribution. The terms
> + * are also available at http://subversion.tigris.org/license-1.html.
> + * If newer versions of this license are posted there, you may use a
> + * newer version instead, at your option.
> + *
> + * This software consists of voluntary contributions made by many
> + * individuals. For exact contribution history, see the revision
> + * history and logs, available at http://subversion.tigris.org/.
> + * ====================================================================
> + */
> +
> +#include <apr_pools.h>
> +
> +#include "svn_types.h"
> +
> +
> +svn_commit_info_t *
> +svn_create_commit_info (apr_pool_t *pool)
> +{
> + svn_commit_info_t *commit_info
> + = apr_pcalloc (pool, sizeof (svn_commit_info_t));
> +
> + commit_info->revision = SVN_INVALID_REVNUM;
> + /* All other fields were initialized to NULL above. */
> +
> + return commit_info;
> +}
We usually allocate based on the variable itself, like this:
sizeof (*commit_info)
That way if we have to rev the type later, to say svn_commit_info2_t,
there's only one line to change -- and no possibility of forgetting to
fix the allocation and thereby getting seg faults :-).
The name "svn_helpers.c" is a bit generic, and the "svn" prefix is
redundant. How would you feel about "types.c" or "constructors.c"
instead? That would be somewhat descriptive, while being generic
enough to be useful for other constructors later.
Thoughts?
-Karl
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org