Re: [Crash-utility] [PATCH] Fix C99 compatibility issues in embedded copy of GDB
On 2023/03/02 14:34, lijiang wrote:
>> Isn't this "int" needed?
>
> It is, but this part is not actually used (the file is more of a macro
> library for bash, I guess, and the use in readline is merely an
> afterthought).
I also have similar concerns. Some upstream changes are not included in this patch, and
these functions are in the current gdb-10.2, for example:
Yes, they might not need to be fixed actually, so my intention is to fix
the minimum parts that Florian's test detected at this time. If we
cannot build crash with a newer gcc, we will fix them or rebase gdb
depending on the situation..
In addition, for the readline part, in fact some early crash
distributions used the
system readline library, and enabled the feature with the option
"--with-system-readline"
in the crash tool.
The advantages are that there are few changes in crash gdb-10.2.patch and can use new
features in the readline library, which can be easy to update in the system.
The disadvantage is that crash may not be compatible with the system readline library
and
need depend on the system readline library.
The current crash-utility is using the embedded readline lib in gdb, maybe we also have
to
face the future changes. This may come with a trade-off.
Yes, agree.
If the upstream crash enables the --with-system-readline, we have to
maintain the compatibility for newer readlines. But we (at least I) are
not very familiar with the gdb to maintain, and rarely rebase it. Given
these, maybe the embedded readline is not so bad, although there will be
some cases like this time..
> The problem is that the upstream patch does not really
apply to the GDB
> 10.2 sources. None of this work is really forward-looking, given that
> crash will eventually have to import a newer GDB version.
True, but there is no plan to update the embedded GDB for now and it
would be better to sync with the upstream code just in case, so I've
updated some hunks like so. Does the attached patch work for you?
Lianbo, I've changed the diff headers as you said, could you check
and test this?
With the attached patch, the test passed. But I still suggest some minor modifications:
[1] add related upstream commits(/gnulib/GDB and glibc/) to patch log
Ah forgot it, thanks. I'll add, is this ok?
Related GDB commits:
0075c53724f7 Impport libiberty commit: 885b6660c17f from gcc mainline.
b4f26d541aa7 Import GNU Readline 8.1
9c9d63b15ad5 gnulib: update to 776af40e0
3f3328b816ee Use startswith more for strncmp function calls.
[2] no need to add new files at the end of "tar xvzmf
gdb-10.2.tar.gz" this time
Yes, I found that we can restore the patched gdb files with this before
checking out the master (unpatched) branch, so kept it.. but if we
change to do so, it should be another patch.
So will remove the tar hunk.
Thanks,
Kazu