[PATCH v10 1/5] PID namespace translation support

[Ákos Uzonyi]

On Sat, 22 Aug 2020 at 21:07, Dmitry V. Levin <ldv at altlinux.org> wrote:
> On Wed, Aug 19, 2020 at 08:16:33PM +0200, Ákos Uzonyi wrote:
[...]
> > +/**
> > + * Iterator function of the proc_data_cache for id translation.
> > + * If the cache contains the id we are looking for, reads the corresponding
> > + * directory in /proc, and if cache is valid, saves the result.
> > + */
> > +static void
> > +proc_data_cache_iterator_fn(void* fn_data, uint64_t key, uint64_t val)
> > +{
> > +     struct translate_id_params *tip = (struct translate_id_params *)fn_data;
> > +     struct proc_data *pd = (struct proc_data *) (uintptr_t) val;
> > +
> > +     if (!pd)
> > +             return;
> > +
> > +     /* Result already found in an earlier iteration */
> > +     if (tip->result_id)
> > +             return;
> > +
> > +     /* Translate from cache */
> > +     tip->pd = pd;
> > +     translate_id_proc_pid(tip, 0);
> > +     if (!tip->result_id)
> > +             return;
> > +
> > +     /* Now translate from actual data in /proc, to check cache validity */
> > +     translate_id_proc_pid(tip, pd->proc_pid);
> > +}
>
> According to the coverage report [1], the last translate_id_proc_pid call
> is not covered by tests as if tip->result_id was always equal to 0.
> Do you have any ideas why?

It took a bit of time to figure it out, but I found the reason for it.
It was caused by a bug, so it's very useful you pointed this out. The
bug was in trie_iterate_keys_node: it didn't work correctly if "end"
was larger than the maximum allowed key. And in pidns.c "pid_max" was
passed as "end" where "pid_max - 1" should have been (this was also a
little bug, but trie should have been able to handle it). I fixed both
bugs, now the line is covered. And also, I extended trie_test to test
trie_iterate_keys_node.

> [...]
> > +     /* Iterate through the cache, find potential proc_data */
> > +     trie_iterate_keys(proc_data_cache, 0, pid_max,
> > +             proc_data_cache_iterator_fn, &tip);
> > +     /* (proc_data_cache_iterator_fn takes care about updating proc_data) */
> > +     if (tip.result_id)
> > +             goto exit;
> > +
> > +     /* No cache helped, read all entries in /proc */
> > +     translate_id_dir(&tip, "/proc", true);
> > +
> > +exit:
>
> Likewise, according to [1], this "goto exit" is somehow not covered by tests
> as if translate_id_dir was always called.

This is just the symptom of the previous problem (by the way,
translate_id_dir was not always called, as there are other "goto exit"
and return statements before).