Re: [PATCH 49/83] builtin/apply: move 'lock_file' global into 'struct apply_state'

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sun, Apr 24, 2016 at 9:33 AM, Christian Couder
<christian.couder@xxxxxxxxx> wrote:
> We cannot have a 'struct lock_file' allocated on the stack, as lockfile.c
> keeps a linked list of all created lock_file structures.

By talking about "the stack" here, I suppose you mean that your
initial idea was to move the global lock_file into cmd_apply() or
something?

> So let's make the 'lock_file' variable a pointer to a 'struct lock_file'
> and let's alloc the struct when needed.
>
> Signed-off-by: Christian Couder <chriscool@xxxxxxxxxxxxx>
> ---
> diff --git a/builtin/apply.c b/builtin/apply.c
> @@ -52,6 +52,12 @@ struct apply_state {
> +       /*
> +        * Since lockfile.c keeps a linked list of all created
> +        * lock_file structures, it isn't safe to free(lock_file).
> +        */
> +       struct lock_file *lock_file;

Is there ever a time when lock_file is removed from the list (such as
upon successful write of the index), in which case it would be safe to
free() this?

> @@ -4515,8 +4521,6 @@ static int write_out_results(struct apply_state *state, struct patch *list)
>         return errs;
>  }
>
> -static struct lock_file lock_file;

Does the static lock_file in build_fake_ancestor() deserve the same
sort of treatment? (I haven't traced the code enough to answer this.)

>  #define INACCURATE_EOF (1<<0)
>  #define RECOUNT                (1<<1)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]