[CRIU] [PATCH 4/6] unix: sysctl -- Preserve max_dgram_qlen value

[Cyrill Gorcunov]

On Thu, Sep 13, 2018 at 02:54:24PM +0100, Dmitry Safonov wrote:
> Hi Cyrill, sorry about late small nits,

Thanks for looking at!

> >
> > +static int unix_conf_op(SysctlEntry ***rconf, size_t *n, int op)
> > +{
> > +       int i, ret = -1, flags = op == CTL_READ ? CTL_FLAGS_OPTIONAL : 0;
> 
> `i' differs by type from n,
> ret is used only in one place, where could be omitted,
> flags? Ewww

It is done on a purpose to match other functions in this file.

> 
> > +       char path[ARRAY_SIZE(unix_conf_entries)][MAX_CONF_UNIX_PATH] = { };
> > +       struct sysctl_req req[ARRAY_SIZE(unix_conf_entries)] = { };
> > +       SysctlEntry **conf = *rconf;
> > +
> > +       if (*n != ARRAY_SIZE(unix_conf_entries)) {
> > +               pr_err("unix: Unexpected entries in config (%u %u)\n",
> > +                      (unsigned)*n, (unsigned)ARRAY_SIZE(unix_conf_entries));
> 
> Maybe %zu and omit additional cast?

It will make like shorted for sure but nothing else :) It won't
chage the code generated. I think we need a more significant
cleanup of the whole criu/net.c file, which I plan to make
and will zap this nit as well :)

> 
> > +               return -EINVAL;
> > +       }
> > +
> > +       if (opts.weak_sysctls)
> 
> Can we do (opts.weak_sysctls || op == CTL_READ)?

Same reason => to match other functions.

> > +                       pr_err("unix: Unknown config type %d\n",
> > +                              (unsigned)conf[i]->type);
> 
> Hehe, %d (unsigned)

Yup, and this is not an error :-)

> > +       if (op == CTL_READ) {
> > +               bool has_entries = false;
> > +
> > +               for (i = 0; i < *n; i++) {
> > +                       if (req[i].flags & CTL_FLAGS_HAS) {
> > +                               conf[i]->has_iarg = true;
> > +                               if (!has_entries)
> > +                                       has_entries = true;
> 
> Is (!has_entries) check here to save processor cache
> from being invalidated? ;-)

:-)