Messages in this thread | | | From | Ammar Faizi <> | Subject | Re: [PATCH 2/2] tools/nolibc: x86-64: Fix startup code bug | Date | Mon, 18 Oct 2021 13:53:29 +0700 |
| |
On Mon, Oct 18, 2021 at 11:59 AM Willy Tarreau <w@1wt.eu> wrote: > > Extra fixes: > > - Use NR_exit_group instead of NR_exit. > > Please, this is independent on the fix above so it must be subject > of a different patch with its own justification. Also it should cover > all supported architectures, otherwise programs will start to behave > differently on different targets. > > > - Use `mov %eax,%edi` instead of `movzbq %al,%rdi`. This makes the > > exit code more observable from strace. While the exit code is > > only 8-bit, the kernel has taken care of that, so no need to > > worry about it. > > I'm fine with this one as well, but similarly, it should be in its own > patch and applied to all architectures. > [...] > > I find the whole thing much less readable here, as asm code tends to > be read as visual groups of patterns. I'm suggesting that you place a > multi-line comment before the asm statement indicating the general > rules (e.g. lowest stack frame must be zero, rsp+8 must be multiple of > 16 etc), then only comment each instruction on the same line as it was > before.
Got it, agree with that. I will address your review and resend this as a patchset v2 soon.
-- Ammar Faizi
| |