[RFC v7 12/21] um: nommu: system call interface and application API

[Octavian Purdila]

On Wed, Oct 7, 2020 at 10:05 PM Johannes Berg <johannes at sipsolutions.net> wrote:
>
> On Tue, 2020-10-06 at 18:44 +0900, Hajime Tazaki wrote:
> >
> > +++ b/arch/um/nommu/include/uapi/asm/syscalls.h
> > @@ -0,0 +1,287 @@
> > +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
>
> That doesn't really make sense - if you use LKL you're linking Linux, so
> you can't use this "WITH Linux-syscall-note"?
>
> > +#ifndef __UM_NOMMU_UAPI_SYSCALLS_H
> > +#define __UM_NOMMU_UAPI_SYSCALLS_H
>
> [snip]
>
> This file really worries me, it includes half the world and (re)defines
> the other half ... How is this ever going to be maintained?
>

There are not that many definitions here, just the ones that were
never defined in uapi headers. And, AFAIK, new code that exposes
structures and data types should always go  into uapi headers and not
directly in glibc, etc. So once we fix the old stuff, it should be
fine?

> > index 000000000000..ec7356c0dee9
> > --- /dev/null
> > +++ b/arch/um/scripts/headers_install.py
> > @@ -0,0 +1,197 @@
> > +#!/usr/bin/env python
>
> might want to make that explicitly 'python3', some newer distros (e.g.
> ubuntu 20.04) are now shipping without a 'python' by default.
>

Good point, will fix it in the next patch series.

> > +def has_lkl_prefix(w):
> > +  return w.startswith("lkl") or w.startswith("_lkl") or w.startswith("__lkl") \
> > +         or w.startswith("LKL") or w.startswith("_LKL") or w.startswith("__LKL")
>
>
> > +        content = re.sub(re.compile("(\/\*(\*(?!\/)|[^*])*\*\/)", re.S|re.M), " ", open(h).read())
> >
> > +    dir = os.path.dirname(h)
> > +    out_dir = args.path + "/" + re.sub("(" + srctree + "/arch/um/nommu/include/uapi/|arch/um/nommu/include/generated/uapi/|include/generated/uapi/|include/generated|" + install_hdr_path + "/include/)(.*)", "lkl/\\2", dir)
>
>
> you have some very long lines in places, I'm sure you could fix that
> (e.g. the last one by doing something with '|'.join([...]))
>

Thanks for pointing it out, we will fix it :)