Quoting Matt Helsley (matthltc@xxxxxxxxxx):
> It looked like a portion of one of these functions was missing
> the block "dispatching" errors so I factored this section out
> and called it from both functions. Was there a good reason it
> was missing?
>
> Signed-off-by: Matt Helsley <matthltc@xxxxxxxxxx>
Good catch, but wouldn't it be better to instead factor the code
from label again: up to
/* if len specified, enforce, else if maximum specified, enforce */
if ((len && h->len != len) || (!len && max && h->len > max))
return -EINVAL;
out of both into one function? This feels more artificial, and I don't
see any other meaningful differences.
-serge
> ---
> checkpoint/restart.c | 49 ++++++++++++++++++++++++++++++++++---------------
> 1 files changed, 34 insertions(+), 15 deletions(-)
>
> diff --git a/checkpoint/restart.c b/checkpoint/restart.c
> index 9b75de8..439206b 100644
> --- a/checkpoint/restart.c
> +++ b/checkpoint/restart.c
> @@ -238,6 +238,31 @@ static int _ckpt_read_objref(struct ckpt_ctx *ctx, struct ckpt_hdr *hh)
> return ret;
> }
>
> +/**
> + * _ckpt_read_early_dispatch - Dispatch ERRORs and OBJREFs; don't return them.
> + * @ctx: checkpoint context
> + * @h: desired ckpt_hdr
> + *
> + * Returns -EFOO, 0 if it succeeded, 1 if h has not been dispatched yet
> + * (so it should be returned rather than ignored)
> + */
> +static int _ckpt_read_early_dispatch(struct ckpt_ctx *ctx, struct ckpt_hdr *h)
> +{
> + int ret;
> +
> + if (h->type == CKPT_HDR_ERROR) {
> + ret = _ckpt_read_err(ctx, h);
> + if (ret < 0)
> + return ret;
> + return 0;
> + } else if (h->type == CKPT_HDR_OBJREF) {
> + ret = _ckpt_read_objref(ctx, h);
> + if (ret < 0)
> + return ret;
> + return 0;
> + }
> + return 1;
> +}
>
> /**
> * _ckpt_read_obj - read an object (ckpt_hdr followed by payload)
> @@ -263,17 +288,11 @@ static int _ckpt_read_obj(struct ckpt_ctx *ctx, struct ckpt_hdr *h,
> if (h->len < sizeof(*h))
> return -EINVAL;
>
> - if (h->type == CKPT_HDR_ERROR) {
> - ret = _ckpt_read_err(ctx, h);
> - if (ret < 0)
> - return ret;
> + ret = _ckpt_read_early_dispatch(ctx, h);
> + if (!ret)
> goto again;
> - } else if (h->type == CKPT_HDR_OBJREF) {
> - ret = _ckpt_read_objref(ctx, h);
> - if (ret < 0)
> - return ret;
> - goto again;
> - }
> + else if (ret < 0)
> + return ret;
>
> /* if len specified, enforce, else if maximum specified, enforce */
> if ((len && h->len != len) || (!len && max && h->len > max))
> @@ -371,12 +390,12 @@ static void *ckpt_read_obj(struct ckpt_ctx *ctx, int len, int max)
> if (hh.len < sizeof(*h))
> return ERR_PTR(-EINVAL);
>
> - if (hh.type == CKPT_HDR_OBJREF) {
> - ret = _ckpt_read_objref(ctx, &hh);
> - if (ret < 0)
> - return ERR_PTR(ret);
> +
> + ret = _ckpt_read_early_dispatch(ctx, &hh);
> + if (!ret)
> goto again;
> - }
> + else if (ret < 0)
> + return ERR_PTR(ret);
>
> /* if len specified, enforce, else if maximum specified, enforce */
> if ((len && hh.len != len) || (!len && max && hh.len > max))
> --
> 1.5.6.3
>
> _______________________________________________
> Containers mailing list
> Containers@xxxxxxxxxxxxxxxxxxxxxxxxxx
> https://lists.linux-foundation.org/mailman/listinfo/containers
_______________________________________________
Containers mailing list
Containers@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linux-foundation.org/mailman/listinfo/containers
