On 2023/03/02 16:30, HAGIO KAZUHITO(萩尾 一仁) wrote:
> 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.

sorry, still missed:

Related glibc commit:
2337e04e21ba cdefs: Limit definition of fortification macros 

>
>> [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.
>