Re: [PATCH] newlib: riscv: Fix build

[Adding Jjohnston]

Hey Jeff,

can you please take a look?  THis concernes the build system...


Thanks,
Corinna


On Mar 27 09:58, Eric Salem wrote:
> On 3/26/25 4:22 PM, Eric Salem wrote:
> > On 3/26/25 8:28 AM, Eric Salem wrote:
> >> On 3/25/25 9:15 PM, Eric Salem wrote:
> >>> Hi Kito,
> >>>
> >>> On 3/25/25 8:39 PM, Kito Cheng wrote:
> >>>> Just one more comment, don't put xlenint.h in
> >>>> newlib/libc/machine/riscv/sys, please move that into
> >>>> newlib/libc/machine/riscv
> >>>> The difference between the two is that the former will be installed
> >>>> into the toolchain, but the latter one won't.
> >>>>
> >>>> Otherwise LGTM, also verified on my side :)
> >>>
> >>> I'm trying to move the file to that directory, but now the problem I'm
> >>> facing is sys/string.h in riscv can no longer find the file. Newlib
> >>> copies the header files to the targ-include directory when building,
> >>> but only those that are in the machine and sys directories. So when I go
> >>> to build, it can't find xlenint.h anymore.
> >>>
> >>> Any suggestions? Perhaps I'm doing something wrong, but looking at the
> >>> file layout of other ports, this seems intentional by Newlib, where the
> >>> libc/machine/<port> directories contain C and assembly files, and all
> >>> header files go in subdirectories.
> >>>
> >>> Eric
> >>
> >> It looks like if I add the path to Makefile.am, this could work. I'll dig
> >> into it today.
> > 
> > I'm a little concerned about making this change. If you run the following
> > find command in newlib/libc/machine:
> > 
> > find -maxdepth 2 -iname "*.h"
> > 
> > You'll see all the header files contained in each newlib/libc/machine/<port>
> > directory:
> > 
> > ./aarch64/asmdefs.h
> > ./amdgcn/exit-value.h
> > ./arc/asm.h
> > ./arm/arm_asm.h
> > ./h8300/defines.h
> > ./h8300/setarch.h
> > ./hppa/DEFS.h
> > ./i386/i386mach.h
> > ./m68k/m68kasm.h
> > ./microblaze/mb_endian.h
> > ./powerpc/fix64.h
> > ./sh/asm.h
> > ./spu/c99ppe.h
> > ./spu/ea_internal.h
> > ./spu/spu_timer_internal.h
> > ./spu/straddr.h
> > ./spu/strcpy.h
> > ./spu/strncmp.h
> > ./spu/vec_literal.h
> > ./visium/memcpy.h
> > ./visium/memset.h
> > ./x86_64/x86_64mach.h
> > ./xtensa/xtensa-asm.h
> > ./z8k/args.h
> > 
> > If Makefile.am is updated to copy all header files in <port> to targ-include,
> > I don't know what the consequences would be for each port, and there's no
> > practical way for me to thoroughly test this.
> > 
> > To reiterate, it's not a C or assembly file in <port> that's including a header
> > file that would also be located in <port>. It's a header file (in this case)
> > in <port>/sys that needs to also include a header file currently located in
> > <port>/sys (currently how the patch would work).
> > 
> > What does everyone think? I think keeping the new xlenint.h header file in
> > <port>/sys would be the safer option.
> > 
> > Eric
> 
> Doing some more digging, when I run:
> 
> find -ipath "*/sys/*.h"
> 
> in newlib/lib/machine, I get some interesting results for spu:
> 
> ./spu/sys/custom_file.h
> ./spu/sys/linux_syscalls.h
> 
> I don't think either of these are supposed to be in the sys directory.
> However, Christian pointed out:
> 
> https://sourceware.org/pipermail/libc-alpha/2017-June/081918.html
> 
> mentions __libc_detect_null() shouldn't be present in a public header. So
> what I'm going to do is move both sys/string.h and sys/xlenint.h one
> directory above, update the includes for the various C and assembly files,
> and send a v2 patch.
> 
> Jeff, if you don't mind, could you test the patch once it's ready? Just
> want to ensure it doesn't break any of your builds.
> 
> Thanks,
> 
> Eric