| Message ID | 20210318180727.116004-1-nikos.nikoleris@arm.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Fix the devicetree parser for stdout-path | expand |
On Thu, Mar 18, 2021 at 06:07:23PM +0000, Nikos Nikoleris wrote: > This set of patches fixes the way we parse the stdout-path > property in the DT. The stdout-path property is used to set up > the console. Prior to this, the code ignored the fact that > stdout-path is made of the path to the uart node as well as > parameters. As a result, it would fail to find the relevant DT > node. In addition to minor fixes in the device tree code, this > series pulls a new version of libfdt from upstream. > > v1: https://lore.kernel.org/kvm/20210316152405.50363-1-nikos.nikoleris@arm.com/ > > Changes in v2: > - Added strtoul and minor fix in strrchr > - Fixes in libfdt_clean > - Minor fix in lib/libfdt/README > > Thanks, > > Nikos > Applied to arm/queue https://gitlab.com/rhdrjones/kvm-unit-tests/-/commits/arm/queue Thanks, drew
On 22/03/2021 08:53, Andrew Jones wrote: > On Thu, Mar 18, 2021 at 06:07:23PM +0000, Nikos Nikoleris wrote: >> This set of patches fixes the way we parse the stdout-path >> property in the DT. The stdout-path property is used to set up >> the console. Prior to this, the code ignored the fact that >> stdout-path is made of the path to the uart node as well as >> parameters. As a result, it would fail to find the relevant DT >> node. In addition to minor fixes in the device tree code, this >> series pulls a new version of libfdt from upstream. >> >> v1: https://lore.kernel.org/kvm/20210316152405.50363-1-nikos.nikoleris@arm.com/ >> >> Changes in v2: >> - Added strtoul and minor fix in strrchr >> - Fixes in libfdt_clean >> - Minor fix in lib/libfdt/README >> >> Thanks, >> >> Nikos >> > > Applied to arm/queue > > https://gitlab.com/rhdrjones/kvm-unit-tests/-/commits/arm/queue > > Thanks, > drew > Thanks for the reviews! Nikos IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
On Mon, 22 Mar 2021 09:53:36 +0100 Andrew Jones <drjones@redhat.com> wrote: > On Thu, Mar 18, 2021 at 06:07:23PM +0000, Nikos Nikoleris wrote: > > This set of patches fixes the way we parse the stdout-path > > property in the DT. The stdout-path property is used to set up > > the console. Prior to this, the code ignored the fact that > > stdout-path is made of the path to the uart node as well as > > parameters. As a result, it would fail to find the relevant DT > > node. In addition to minor fixes in the device tree code, this > > series pulls a new version of libfdt from upstream. > > > > v1: https://lore.kernel.org/kvm/20210316152405.50363-1-nikos.nikoleris@arm.com/ > > > > Changes in v2: > > - Added strtoul and minor fix in strrchr > > - Fixes in libfdt_clean > > - Minor fix in lib/libfdt/README > > > > Thanks, > > > > Nikos > > > > Applied to arm/queue So I understand that this is a bit late now, but is this really the way forward: to just implement libc functions as we go, from scratch, and merge them without any real testing? I understand that hacking up strchr() is fun, but when it comes to those string parsing functions, it gets a bit hairy, and I feel like we are dismissing decades of experience here by implementing stuff from scratch. At the very least we should run some unit tests (!) on newly introduced C library functions? Or probably the better alternative: we pick some existing C library, and start to borrow implementations from there? Is klibc[1] a good choice, maybe? Cheers, Andre [1] https://git.kernel.org/pub/scm/libs/klibc/klibc.git/ > > https://gitlab.com/rhdrjones/kvm-unit-tests/-/commits/arm/queue > > Thanks, > drew >
On Mon, Mar 22, 2021 at 06:04:45PM +0000, Andre Przywara wrote: > On Mon, 22 Mar 2021 09:53:36 +0100 > Andrew Jones <drjones@redhat.com> wrote: > > > On Thu, Mar 18, 2021 at 06:07:23PM +0000, Nikos Nikoleris wrote: > > > This set of patches fixes the way we parse the stdout-path > > > property in the DT. The stdout-path property is used to set up > > > the console. Prior to this, the code ignored the fact that > > > stdout-path is made of the path to the uart node as well as > > > parameters. As a result, it would fail to find the relevant DT > > > node. In addition to minor fixes in the device tree code, this > > > series pulls a new version of libfdt from upstream. > > > > > > v1: https://lore.kernel.org/kvm/20210316152405.50363-1-nikos.nikoleris@arm.com/ > > > > > > Changes in v2: > > > - Added strtoul and minor fix in strrchr > > > - Fixes in libfdt_clean > > > - Minor fix in lib/libfdt/README > > > > > > Thanks, > > > > > > Nikos > > > > > > > Applied to arm/queue > > So I understand that this is a bit late now, but is this really the way > forward: to just implement libc functions as we go, from scratch, and > merge them without any real testing? > I understand that hacking up strchr() is fun, but when it comes to > those string parsing functions, it gets a bit hairy, and I feel like we > are dismissing decades of experience here by implementing stuff from > scratch. At the very least we should run some unit tests (!) on newly > introduced C library functions? Who says I didn't test the new string functions? Did you come up with a test case that breaks something? > > Or probably the better alternative: we pick some existing C library, > and start to borrow implementations from there? Is klibc[1] a good > choice, maybe? The trivial functions like strchr don't really scare me much. And the more complicated functions don't always adapt to our framework. I just looked at klibc's strtol. It doesn't have any error handling; not the errno type specified in the man page and not the type we do in kvm-unit-tests (asserts). IOW, our implementation is even more complete. Anyway, after like 15 years of development, kvm-unit-tests only has 20 string, 3 stdlib, and 4 printf functions. I'm not too worried about overly reinventing the wheel just yet :-) Thanks, drew > > Cheers, > Andre > > [1] https://git.kernel.org/pub/scm/libs/klibc/klibc.git/ > > > > > > https://gitlab.com/rhdrjones/kvm-unit-tests/-/commits/arm/queue > > > > Thanks, > > drew > > >