Re: [PATCH 1/5] Linux: Add tables with system call numbers

[Siddhesh Poyarekar <siddhesh@gotplt.org>]

On 31/12/19 4:52 pm, Florian Weimer wrote:
>> Prefix the auto-generated files with a comment header like so:
>>
>> /* AUTOGENERATED by update-syscall-lists.py. DO NOT EDIT. */
> 
> I'm concerned that this unnecessarily makes processing by tools more
> difficult, so I'd like to avoid this.

The tools only need to filter out \/\*.*\*\/ (or een simpler, omit the
first line if it's just for these files) which shouldn't be more than a
line or two of code.  Do you see any other challenges here?  It's a very
useful comment IMO and definitely worth the extra line of code one would
have to add to omit comments or even the first line.

> I copied the linux_kernel_version function from
> sysdeps/unix/sysv/linux/tst-mman-consts.py, which is why I kept the
> 2018 year.

OK.

>>> +    # Merge the architecture-specific system call names into the
>>> +    # global names list, syscall-names.list.  This file contains names
>>> +    # from other architectures (and comments), so it is necessary to
>>> +    # merge the existing files with the names obtained from the
>>> +    # kernel.
>>> +    with open(args.names_list, "r+") as list_file:
>>> +        os.lockf(list_file.fileno(), os.F_LOCK, 0)
>>> +        names_list = glibcsyscalls.SyscallNamesList(list_file)
>>> +        merged = names_list.merge(kernel_constants.keys())
>>> +        list_file.truncate()
>>> +        list_file.seek(0)
>>> +        for line in merged:
>>> +            list_file.write(line)
>>
>> I think the cleaner way would be to write out the output as a temp first
>> and then rename over the old file.  This ought to replicate what we do
>> in Makefiles.
> 
> But we need locking if this is used from build-many-glibcs.py,
> otherwise we'll lose updates even with the rename approach.  Locking
> the file being updated is the easiest way to achieve this.  It's true
> that we have data loss if the script is terminated between the
> truncate and seek, but most editors have the same issue.  Therefore, I
> want to leave this as-is.

It would be nice to make build-many-glibcs.py smarter about this.  The
problem with locking seems limited to syscall-names.list and not
arch-syscalls.h since the latter ought to be different files for
build-many-glibcs, right?

I don't want to hold up this patchset for it, but want to make sure we
at least fully understand and document the limitations for posterity.

> New patch below.  Commit message is unchanged.
> 

Rest looks OK, so lets find common ground on the above two points.

Siddhesh