[augeas-devel] [PATCH] Allocate exception instead of static const value

[David Lutterkort]

Hi Francis,

I just committed a variation of this patch as commit 4acd1f9c. The main
change I made was keep the fatal exception in struct error, rather than
in a global variable. Since the main reason for this change is that the
backup exn might be modified by code as it is thrown, I didn't want to
run the risk of threads clobbering each other.

David

On Fri, 2012-01-13 at 13:12 -0500, Francis Giraldeau wrote:
> The static const value returned by exn_error() may cause SIGSEGV in case the
> value is further changed. To avoid this situation, the structure is allocated
> early in aug_init instead and released in aug_close. The resulting exception
> value can be modified without adverse effect.
> 
> Only one global exception value is allocated by process. The value is shared
> between aug instances of the same process, since it is used only as a last
> resort.
> ---
>  src/augeas.c |    2 ++
>  src/syntax.c |   33 ++++++++++++++++++++++++---------
>  src/syntax.h |    2 ++
>  3 files changed, 28 insertions(+), 9 deletions(-)
> 
> diff --git a/src/augeas.c b/src/augeas.c
> index e39208a..733a11b 100644
> --- a/src/augeas.c
> +++ b/src/augeas.c
> @@ -522,6 +522,7 @@ struct augeas *aug_init(const char *root, const char *loadpath,
>      if (result->error->info->filename == NULL)
>          goto error;
>      result->error->aug = result;
> +    init_fatal_exn();
>  
>      result->origin = make_tree_origin(tree_root);
>      if (result->origin == NULL) {
> @@ -1702,6 +1703,7 @@ void aug_close(struct augeas *aug) {
>      free(aug->error->details);
>      free(aug->error);
>      free(aug);
> +    release_fatal_exn();
>  }
>  
>  int __aug_load_module_file(struct augeas *aug, const char *filename) {
> diff --git a/src/syntax.c b/src/syntax.c
> index 320d588..3f5aacc 100644
> --- a/src/syntax.c
> +++ b/src/syntax.c
> @@ -81,6 +81,29 @@ struct ctx {
>      struct binding *local;
>  };
>  
> +/* fatal_exn: static pointer for unrecoverable conditions */
> +static struct value *fatal_exn = NULL;
> +
> +void init_fatal_exn(void) {
> +    if (fatal_exn != NULL)
> +        return;
> +    fatal_exn = make_exn_value(NULL, "Error during evaluation");
> +    if (fatal_exn == NULL)
> +        return;
> +    fatal_exn->exn->seen = 1;
> +    fatal_exn->exn->error = 1;
> +    fatal_exn->exn->lines = NULL;
> +    fatal_exn->exn->nlines = 0;
> +    fatal_exn->ref = REF_MAX;
> +}
> +
> +void release_fatal_exn(void) {
> +    if (fatal_exn == NULL)
> +        return;
> +    fatal_exn->ref = 1;
> +    unref(fatal_exn, value);
> +}
> +
>  static void format_error(struct info *info, aug_errcode_t code,
>                           const char *format, va_list ap) {
>      struct error *error = info->error;
> @@ -435,15 +458,7 @@ void exn_printf_line(struct value *exn, const char *format, ...) {
>  }
>  
>  struct value *exn_error(void) {
> -    static const struct exn exn = {
> -        .info = NULL, .seen = 1, .error = 1,
> -        .message = (char *) "Error during evaluation",
> -        .nlines = 0, .lines = NULL };
> -    static const struct value value = {
> -        .ref = REF_MAX, /* Protect against being freed */
> -        .info = NULL, .tag = V_EXN,
> -        { .exn = (struct exn *) &exn } };
> -    return (struct value *) &value;
> +    return fatal_exn;
>  }
>  
>  /*
> diff --git a/src/syntax.h b/src/syntax.h
> index 4c517de..bf9da6f 100644
> --- a/src/syntax.h
> +++ b/src/syntax.h
> @@ -173,6 +173,8 @@ struct value {
>  };
>  
>  /* Return an exception value with error == 1 (and seen == 1) */
> +void init_fatal_exn(void);
> +void release_fatal_exn(void);
>  struct value *exn_error(void);
>  
>  /* All types except for T_ARROW (functions) are simple. Subtype relations