Madan U Sreenivasan <madan@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
Received on Thu Aug 25 05:42:44 2005