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.