On Tue, Feb 10, 2026 at 06:27:49PM -0500, Nicholas Vinson wrote:
> On 2/10/26 10:47, Daniel Kiper wrote:
> > On Mon, Feb 09, 2026 at 07:43:28PM -0500, Nicholas Vinson wrote:
> > > Starting with ld.llvm-21, any attempt create a non-relocatable binary and 
> > > set
> > > one more secton addresses below 0x400000 results in a linker error. 
> > > Furthermore,
> > > the differences between ld.bfd and ld.lld made finding a proper set of
> > > command-line flags tht worked with both linkers and bypass the image base
> > > address restriction difficult. Therefore, the approach of using a custom 
> > > linker
> > > script was adopted to solve the issue.
> > >
> > > This approach was tested using:
> > >
> > > ../configure CC=clang CXX=clang++ LDFLAGS="-fuse-ld=lld" 
> > > TARGET_LDFLAGS="-fuse-ld=lld" --with-platform=pc
> > > ../configure CC=clang CXX=clang++ --with-platform=pc (both with ld.lld as 
> > > the default and ld.bfd as the default)
> > > ../configure CC=gcc CXX=g++ --with-platform=pc
> > >
> > > and a VM was used for testing. To build the disk images the VM was booted 
> > > with,
> > > the following scripts were used:
> >
> > [...]
> >
> > > In all cases, the VM successfully booted to the standard GRUB prompt.
> > >
> > > Nicholas Vinson (8):
> > >    i386/pc/int.h: conditionally apply regparm attr.
> > >    grub-core: Update kernel image generation
> > >    i386-cygwin-img-ld.sc -> i386-cygwin-img.lds
> > >    Revert "configure: Print a more helpful error if autoconf-archive is
> > >      not installed"
> > >    Revert "configure: Check linker for --image-base support"
> > >    Revert "INSTALL: Add note that the GNU Autoconf Archive may be needed"
> > >    configure: drop -Ttext checks for i386-pc
> >
> > For all these patches Reviewed-by: Daniel Kiper <[email protected]>...
> >
> > >    C23 fixes: fix strchr() and strrchr() handling
> >
> > It seems to me that this patch does more than commit message says.
> > I think it has to be split into more parts or commit message has to be
> > improved.
>
> I admit the message is a bit terse, but I kept the changes to a minimum. I
> did, however, forget to mention strstr().

If you mention strstr() I am OK with it...

> Put simply, with C23, if the first argument to strstr(), strchr(), or
> strrchar() is a const char *, the result is a const char *. C23 also made
> this change to about 9 or so other functions.
>
> As a result, most of the changes were just changing a variable from 'char *'
> to 'const char *'.
>
> The changes for ofpath.c are a bit more involved because the original code
> would modify the string 'ed' pointed to. With C23, ed needs to be a 'const
> char *' because sysfs_path is a 'const char *'. This also means that the
> line "*q = '\0'" is no longer valid because you cannot safely modify a const
> char string.
>
> I saw two options to fix this, the first was to modify the line to be q =
> strndup(...);, add the requisite error checking, and then make sure to free
> q just before every subsequent return. The second option, is what I did,
> find the difference between q and ed, save it off, and then replace the
> strlen(ed) calls with that difference.
>
> I believe the second approach is the better of the two. It did have the
> ancillary effect of updating a few snprintf() calls and the path_size
> calculation; however, these changes are required with this approach.

I am OK with second approach but I think this change should go to
separate patch, with an explanation like above, because it is more
complicated than just adding const.

> If you'd prefer, I can re-submit patch 8 with these details in the commit
> message. Alternatively, if you would prefer the strndup() approach, I will
> respin this patch series without patch 8, and then submit a re-worked C23
> patch as a separate request.

Just split patch #8 as I suggested above and repost C23 as separate series.

Daniel

_______________________________________________
Grub-devel mailing list
[email protected]
https://lists.gnu.org/mailman/listinfo/grub-devel

Reply via email to