Re: [PATCH 6/4] libbacktrace: Add loaded dlls after initialize
> From: Ian Lance Taylor
> Date: Mon, 29 Jul 2024 09:46:46 -0700
> Cc: Eli Zaretskii , gcc-patches@gcc.gnu.org, g...@gcc.gnu.org
>
> On Fri, Mar 15, 2024 at 1:41 PM Björn Schäpers wrote:
> >
> > Am 10.01.2024 um 13:34 schrieb Eli Zaretskii:
> > >> Date: Tue, 9 Jan 2024 21:02:44 +0100
> > >> Cc: i...@google.com, gcc-patches@gcc.gnu.org, g...@gcc.gnu.org
> > >> From: Björn Schäpers
> > >>
> > >> Am 07.01.2024 um 18:03 schrieb Eli Zaretskii:
> > >>> In that case, you an call either GetModuleHandeExA or
> > >>> GetModuleHandeExW, the difference is minor.
> > >>
> > >> Here an updated version without relying on TEXT or TCHAR, directly
> > >> calling
> > >> GetModuleHandleExW.
> > >
> > > Thanks, this LGTM (but I couldn't test it, I just looked at the
> > > sour ce code).
> >
> > Here an updated version. It is rebased on the combined approach of getting
> > the
> > loaded DLLs and two minor changes to suppress warnings.
>
> This bug report was filed about this patch:
>
> https://github.com/ianlancetaylor/libbacktrace/issues/131
>
> > src\pecoff.c(86): error C2059: syntax error: '('
> > src\pecoff.c(89): error C2059: syntax error: '('
> >
> > It works fine if deleting CALLBACK and NTAPI.
>
> Any ideas?
Instead of deleting those, move them inside the parentheses:
typedef VOID (CALLBACK *LDR_DLL_NOTIFICATION)(ULONG,
struct dll_notification_data*,
PVOID);
typedef NTSTATUS (NTAPI *LDR_REGISTER_FUNCTION)(ULONG,
LDR_DLL_NOTIFICATION, PVOID,
PVOID*);
and also I think you need to include , for the definition
of the NTSTATUS type.
Caveat: I don't have MSVC, so I couldn't verify that these measures
fix the problem, sorry.
Re: [PATCH 6/4] libbacktrace: Add loaded dlls after initialize
> Date: Tue, 9 Jan 2024 21:02:44 +0100 > Cc: i...@google.com, gcc-patches@gcc.gnu.org, g...@gcc.gnu.org > From: Björn Schäpers > > Am 07.01.2024 um 18:03 schrieb Eli Zaretskii: > > In that case, you an call either GetModuleHandeExA or > > GetModuleHandeExW, the difference is minor. > > Here an updated version without relying on TEXT or TCHAR, directly calling > GetModuleHandleExW. Thanks, this LGTM (but I couldn't test it, I just looked at the sour ce code).
Re: [PATCH 6/4] libbacktrace: Add loaded dlls after initialize
> Date: Sun, 7 Jan 2024 17:07:06 +0100 > Cc: i...@google.com, gcc-patches@gcc.gnu.org, g...@gcc.gnu.org > From: Björn Schäpers > > > That was about GetModuleHandle, not about GetModuleHandleEx. For the > > latter, all Windows versions that support it also support "wide" APIs. > > So my suggestion is to use GetModuleHandleExW here. However, you will > > need to make sure that notification_data->dll_base is declared as > > 'wchar_t *', not 'char *'. If dll_base is declared as 'char *', then > > only GetModuleHandleExA will work, and you will lose the ability to > > support file names with non-ASCII characters outside of the current > > system codepage. > > The dll_base is a PVOID. With the GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS flag > GetModuleHandleEx does not look for a name, but uses an adress in the module > to > get the HMODULE, so you cast it to char* or wchar_t* depending on which > function > you call. Actually one could just cast the dll_base to HMODULE, at least in > win32 on x86 the HMODULE of a dll is always its base adress. But to make it > safer and future proof I went the way through GetModuleHandeEx. In that case, you an call either GetModuleHandeExA or GetModuleHandeExW, the difference is minor.
Re: [PATCH 6/4] libbacktrace: Add loaded dlls after initialize
[I re-added the other addressees, as I don' think you meant to make this discussion private between the two of us.] > Date: Sun, 7 Jan 2024 12:58:29 +0100 > From: Björn Schäpers > > Am 07.01.2024 um 07:50 schrieb Eli Zaretskii: > >> Date: Sat, 6 Jan 2024 23:15:24 +0100 > >> From: Björn Schäpers > >> Cc: gcc-patches@gcc.gnu.org, g...@gcc.gnu.org > >> > >> This patch adds libraries which are loaded after backtrace_initialize, like > >> plugins or similar. > >> > >> I don't know what style is preferred for the Win32 typedefs, should the > >> code use > >> PVOID or void*? > > > > It doesn't matter, at least not if the source file includes the > > Windows header files (where PVOID is defined). > > > >> + if (reason != /*LDR_DLL_NOTIFICATION_REASON_LOADED*/1) > > > > IMO, it would be better to supply a #define if undefined: > > > > #ifndef LDR_DLL_NOTIFICATION_REASON_LOADED > > # define LDR_DLL_NOTIFICATION_REASON_LOADED 1 > > #endif > > > > I surely can define it. But the ifndef is not needed, since there are no > headers > containing the function signatures, structures or the defines: > https://learn.microsoft.com/en-us/windows/win32/devnotes/ldrregisterdllnotification OK, I wasn't sure about that. > >> + if (!GetModuleHandleEx (GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS > >> +| GET_MODULE_HANDLE_EX_FLAG_UNCHANGED_REFCOUNT, > >> +(TCHAR*) notification_data->dll_base, > > > > Is TCHAR correct here? does libbacktrace indeed use TCHAR and relies > > on compile-time definition of UNICODE? (I'm not familiar with the > > internals of libbacktrace, so apologies if this is a silly question.) > > > > Thanks. > > As far as I can see it's the first time for TCHAR, I would've gone for > GetModuleHandleExW, but > https://gcc.gnu.org/pipermail/gcc/2023-January/240534.html That was about GetModuleHandle, not about GetModuleHandleEx. For the latter, all Windows versions that support it also support "wide" APIs. So my suggestion is to use GetModuleHandleExW here. However, you will need to make sure that notification_data->dll_base is declared as 'wchar_t *', not 'char *'. If dll_base is declared as 'char *', then only GetModuleHandleExA will work, and you will lose the ability to support file names with non-ASCII characters outside of the current system codepage. > But I didn't want to force GetModuleHandleExA, so I went for TCHAR and > GetModuleHandleEx so it automatically chooses which to use. Same for > GetModuleHandle of ntdll.dll. The considerations for GetModuleHandle and for GetModuleHandleEx are different: the former is also available on old versions of Windows that doesn't support "wide" APIs.
Re: [PATCH 6/4] libbacktrace: Add loaded dlls after initialize
> Date: Sat, 6 Jan 2024 23:15:24 +0100 > From: Björn Schäpers > Cc: gcc-patches@gcc.gnu.org, g...@gcc.gnu.org > > This patch adds libraries which are loaded after backtrace_initialize, like > plugins or similar. > > I don't know what style is preferred for the Win32 typedefs, should the code > use > PVOID or void*? It doesn't matter, at least not if the source file includes the Windows header files (where PVOID is defined). > + if (reason != /*LDR_DLL_NOTIFICATION_REASON_LOADED*/1) IMO, it would be better to supply a #define if undefined: #ifndef LDR_DLL_NOTIFICATION_REASON_LOADED # define LDR_DLL_NOTIFICATION_REASON_LOADED 1 #endif > + if (!GetModuleHandleEx (GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS > + | GET_MODULE_HANDLE_EX_FLAG_UNCHANGED_REFCOUNT, > + (TCHAR*) notification_data->dll_base, Is TCHAR correct here? does libbacktrace indeed use TCHAR and relies on compile-time definition of UNICODE? (I'm not familiar with the internals of libbacktrace, so apologies if this is a silly question.) Thanks.
Re: [PATCH 4/4] libbacktrace: get debug information for loaded dlls
> Date: Thu, 30 Nov 2023 11:53:54 -0800 > Cc: gcc-patches@gcc.gnu.org, g...@gcc.gnu.org > From: Ian Lance Taylor via Gcc > > Also starting with a module count of 1000 seems like a lot. Do > typical Windows programs load that many modules? Unlikely. I'd start with 100.
Re: [PATCH 3/4] libbacktrace: work with aslr on windows
> Date: Mon, 20 Nov 2023 20:57:38 +0100 > Cc: gcc-patches@gcc.gnu.org, g...@gcc.gnu.org > From: Björn Schäpers > > +#ifndef NOMINMAX > +#define NOMINMAX > +#endif Why is this part needed? Otherwise, LGTM, thanks. (But I'm don't have the approval rights, so please wait for Ian to chime in.)
Re: RFC: Top level configure: Require a minimum version 6.8 texinfo
> Date: Tue, 29 Aug 2023 17:45:20 +0200 > Cc: gcc-patches@gcc.gnu.org, gdb-patc...@sourceware.org, > binut...@sourceware.org > From: Jakub Jelinek via Gdb-patches > > On Tue, Aug 29, 2023 at 04:21:44PM +0100, Nick Clifton via Gcc-patches wrote: > > Currently the top level configure.ac file sets the minimum required > > version of texinfo to be 4.7. I would like to propose changing this > > to 6.8. > > > > The reason for the change is that the bfd documentation now needs at > > least version 6.8 in order to build[1][2]. Given that 4.7 is now > > almost 20 years old (it was released in April 2004), updating the > > requirement to a newer version does seem reasonable. On the other > > hand 6.8 is quite new (it was released in March 2021), so a lot of > > systems out there may not have it. > > > > Thoughts ? > > I think that is too new. It _is_ new. But I also don't understand why Nick thinks he needs Texinfo 6.8. AFAIR, makeinfo supported @node lines without explicit pointers since at least version 4.8. I have on my disk the manual produced for Emacs 22.1, where the Texinfo sources have no pointers, e.g.: @node Abbrev Concepts and the corresponding Info file says: This is ../info/emacs, produced by makeinfo version 4.8 from emacs.texi. So I'm not sure what exactly is the feature that requires Texinfo 6.8. What am I missing?
Re: [PATCH 2/4] libbacktrace: detect executable path on windows
> From: Ian Lance Taylor > Date: Tue, 24 Jan 2023 09:58:10 -0800 > Cc: g...@hazardy.de, gcc-patches@gcc.gnu.org, g...@gcc.gnu.org > > I'd rather that the patch look like the appended. Can someone with a > Windows system test to see what that builds and passes the tests? ENOPATCH
Re: [PATCH 2/4] libbacktrace: detect executable path on windows
> From: Ian Lance Taylor > Date: Tue, 24 Jan 2023 06:35:21 -0800 > Cc: g...@hazardy.de, gcc-patches@gcc.gnu.org, g...@gcc.gnu.org > > > > On Windows it seems that MAX_PATH is not > > > a true limit, as an extended length path may be up to 32767 bytes. > > > > The limit of 32767 characters (not bytes, AFAIK) is only applicable > > when using the Unicode (a.k.a. "wide") versions of the Windows Win32 > > APIs, see > > > > > > https://learn.microsoft.com/en-us/windows/win32/fileio/maximum-file-path-limitation > > > > Since the above code uses GetModuleFileNameA, which is an "ANSI" > > single-byte API, it is still subject to the MAX_PATH limitation, and > > MAX_PATH is defined as 260 on Windows headers. > > Thanks. Should this code be using GetModuleFileNameW? Or would that > mean that the later call to open will fail? We'd need to use _wopen or somesuch, and the file name will have to be a wchar_t array, not a char array, yes. So this is not very practical when file names need to be passed between functions, unless they are converted to UTF-8 (and back again before using them in Windows APIs). And note that even then, the 260-byte limit could be lifted only if the user has a new enough Windows version _and_ has opted in to the long-name feature by turning it on in the Registry. Otherwise, file names used in "wide" APIs can only break the 260-byte limit if they use the special format "\\?\D:\foo\bar", which means file names specified by user outside of the program or file names that come from other programs will need to be reformatted to this special format. > 260 bytes does not seem like very much for a path name these days. That's true. But complications with using longer file names are still a PITA on Windows, even though they are a step closer to practically possible.
Re: [PATCH 2/4] libbacktrace: detect executable path on windows
> Date: Mon, 23 Jan 2023 15:00:56 -0800
> Cc: gcc-patches@gcc.gnu.org, g...@gcc.gnu.org
> From: Ian Lance Taylor via Gcc
>
> > +#ifdef HAVE_WINDOWS_H
> > +
> > +static char *
> > +windows_get_executable_path (char *buf, backtrace_error_callback
> > error_callback,
> > +void *data)
> > +{
> > + if (GetModuleFileNameA (NULL, buf, MAX_PATH - 1) == 0)
> > +{
> > + error_callback (data,
> > + "could not get the filename of the current
> > executable",
> > + (int) GetLastError ());
> > + return NULL;
> > +}
> > + return buf;
> > +}
>
> Thanks, but this seems incomplete. The docs for GetModuleFileNameA
> say that if the pathname is too long to fit into the buffer it returns
> the size of the buffer and sets the error to
> ERROR_INSUFFICIENT_BUFFER. It seems to me that in that case we should
> allocate a larger buffer and try again.
This is correct in general, but not in this particular case.
> On Windows it seems that MAX_PATH is not
> a true limit, as an extended length path may be up to 32767 bytes.
The limit of 32767 characters (not bytes, AFAIK) is only applicable
when using the Unicode (a.k.a. "wide") versions of the Windows Win32
APIs, see
https://learn.microsoft.com/en-us/windows/win32/fileio/maximum-file-path-limitation
Since the above code uses GetModuleFileNameA, which is an "ANSI"
single-byte API, it is still subject to the MAX_PATH limitation, and
MAX_PATH is defined as 260 on Windows headers.
Re: [PATCH 3/4] libbacktrace: work with aslr on windows
> Date: Sat, 21 Jan 2023 11:47:42 +0100 > Cc: g...@hazardy.de, gcc-patches@gcc.gnu.org, g...@gcc.gnu.org > From: Gabriel Ravier > > > On 1/21/23 05:05, Eli Zaretskii wrote: > >> Date: Fri, 20 Jan 2023 21:39:56 +0100 > >> Cc: g...@hazardy.de, gcc-patches@gcc.gnu.org, g...@gcc.gnu.org > >> From: Gabriel Ravier > >> > >>>> - using wide APIs with Windows is generally considered to be a best > >>>> practice, even when not strictly needed (and in this case I can't see > >>>> any problem with doing so, unless maybe we want to code to work with > >>>> Windows 95 or something like that...) > >>> There's no reason to forcibly break GDB on platforms where wide APIs > >>> are not available. > >> Are there even any platforms that have GetModuleHandleA but not > >> GetModuleHandleW ? MSDN states that Windows XP and Windows Server 2003 > >> are the first versions to support both of the APIs, so if this is > >> supposed to work on Windows 98, for instance, whether we're using > >> GetModuleHandleA or GetModuleHandleW won't matter. > > I'm not sure I follow the logic. A program that calls > > GetModuleHandleW will refuse to start on Windows that doesn't have > > that API. So any version before XP is automatically excluded the > > moment you use code which calls that API directly (i.e. not through a > > function pointer or somesuch). > A program that calls GetModuleHandleA will also refuse to start on > Windows if it doesn't have that API. The set of Windows versions that do > not have GetModuleHandleA is, according to MSDN, the same as the set of > Windows versions that do not have GetModuleHandleW. MSDN lies (because it wants to pretend that older versions don't exist). Try this much more useful site: http://winapi.freetechsecrets.com/win32/WIN32GetModuleHandle.htm
Re: [PATCH 3/4] libbacktrace: work with aslr on windows
> Date: Sat, 21 Jan 2023 17:18:14 +0800 > Cc: g...@hazardy.de, gcc-patches@gcc.gnu.org, g...@gcc.gnu.org > From: LIU Hao > > 在 2023-01-21 12:05, Eli Zaretskii via Gcc 写道: > > I'm not sure I follow the logic. A program that calls > > GetModuleHandleW will refuse to start on Windows that doesn't have > > that API. So any version before XP is automatically excluded the > > moment you use code which calls that API directly (i.e. not through a > > function pointer or somesuch). > > Are _you_ still willing to maintain backward compatibility with Windows 9x? > Even mingw-w64 has been > defaulting to Windows Server 2003 since 2007. Why would anyone build a modern > compiler for such old > operating systems? I'm only saying that we should not deliberately break those old platforms unless we have a good reason. And I see no such good reason in this case: GetModuleHandleA will do the job exactly like GetModuleHandleW will. > With any Windows that is modern enough, wide APIs should always be preferred > to ANSI ones, > especially when the argument is constant. Almost all ANSI APIs (the only > exception I know of is > `OutputDebugStringA` which does the inverse) translate their ANSI string > arguments to wide strings > and delegate to wide ones, so by calling wide APIs explicitly, such overhead > can be avoided. The overhead is only relevant in code that is run in performance critical places. I don't think this is such a place.
Re: [PATCH 3/4] libbacktrace: work with aslr on windows
> Date: Fri, 20 Jan 2023 21:39:56 +0100 > Cc: g...@hazardy.de, gcc-patches@gcc.gnu.org, g...@gcc.gnu.org > From: Gabriel Ravier > > >> - using wide APIs with Windows is generally considered to be a best > >> practice, even when not strictly needed (and in this case I can't see > >> any problem with doing so, unless maybe we want to code to work with > >> Windows 95 or something like that...) > > There's no reason to forcibly break GDB on platforms where wide APIs > > are not available. > Are there even any platforms that have GetModuleHandleA but not > GetModuleHandleW ? MSDN states that Windows XP and Windows Server 2003 > are the first versions to support both of the APIs, so if this is > supposed to work on Windows 98, for instance, whether we're using > GetModuleHandleA or GetModuleHandleW won't matter. I'm not sure I follow the logic. A program that calls GetModuleHandleW will refuse to start on Windows that doesn't have that API. So any version before XP is automatically excluded the moment you use code which calls that API directly (i.e. not through a function pointer or somesuch).
Re: [PATCH 3/4] libbacktrace: work with aslr on windows
> Date: Fri, 20 Jan 2023 17:46:59 +0100 > Cc: gcc-patches@gcc.gnu.org, g...@gcc.gnu.org > From: Gabriel Ravier > > On 1/20/23 14:39, Eli Zaretskii via Gcc wrote: > >> From: Björn Schäpers > >> Date: Fri, 20 Jan 2023 11:54:08 +0100 > >> > >> @@ -856,7 +870,12 @@ coff_add (struct backtrace_state *state, int > >> descriptor, > >> + (sections[i].offset - min_offset)); > >> } > >> > >> - if (!backtrace_dwarf_add (state, /* base_address */ 0, &dwarf_sections, > >> +#ifdef HAVE_WINDOWS_H > >> +module_handle = (uintptr_t) GetModuleHandleW (NULL); > >> +base_address = module_handle - image_base; > >> +#endif > >> + > >> + if (!backtrace_dwarf_add (state, base_address, &dwarf_sections, > >>0, /* FIXME: is_bigendian */ > >>NULL, /* altlink */ > >>error_callback, data, fileline_fn, > > Why do you force using the "wide" APIs here? Won't GetModuleHandle do > > the job, whether it resolves to GetModuleHandleA or GetModuleHandleW? > > I would expect the reason to be either that: > > - using wide APIs with Windows is generally considered to be a best > practice, even when not strictly needed (and in this case I can't see > any problem with doing so, unless maybe we want to code to work with > Windows 95 or something like that...) There's no reason to forcibly break GDB on platforms where wide APIs are not available. > - using the narrow API somehow has an actual drawback, for example maybe > it might not work if the name of the exe file the NULL will tell it to > get a handle to contains wide characters Native Windows port of GDB doesn't support Unicode file names anyway, which is why you used the *A APIs elsewhere in the patch, and rightfully so. So there's no reason to use "wide" APIs in this one place, and every reason not to.
Re: [PATCH 3/4] libbacktrace: work with aslr on windows
> From: Björn Schäpers > Date: Fri, 20 Jan 2023 11:54:08 +0100 > > @@ -856,7 +870,12 @@ coff_add (struct backtrace_state *state, int descriptor, > + (sections[i].offset - min_offset)); > } > > - if (!backtrace_dwarf_add (state, /* base_address */ 0, &dwarf_sections, > +#ifdef HAVE_WINDOWS_H > +module_handle = (uintptr_t) GetModuleHandleW (NULL); > +base_address = module_handle - image_base; > +#endif > + > + if (!backtrace_dwarf_add (state, base_address, &dwarf_sections, > 0, /* FIXME: is_bigendian */ > NULL, /* altlink */ > error_callback, data, fileline_fn, Why do you force using the "wide" APIs here? Won't GetModuleHandle do the job, whether it resolves to GetModuleHandleA or GetModuleHandleW?
Re: Benefits of using Sphinx documentation format
> From: Jonathan Wakely > Date: Mon, 12 Jul 2021 15:54:49 +0100 > Cc: Martin Liška , > "g...@gcc.gnu.org" , gcc-patches > , > "Joseph S. Myers" > > You like texinfo. We get it. Would you please drop the attitude?
Re: Benefits of using Sphinx documentation format
> Cc: h...@bitrange.com, g...@gcc.gnu.org, gcc-patches@gcc.gnu.org, > jos...@codesourcery.com > From: Martin Liška > Date: Mon, 12 Jul 2021 16:37:00 +0200 > > > 4) The need to learn yet another markup language. > > While this is not a problem for simple text, it does require a > > serious study of RST and Sphinx to use the more advanced features. > > No, majority of the documentation is pretty simple: basic formatting, links, > tables and > code examples. We also have documentation of APIs (a.k.a. "functions"). I actually tried to find in the Sphinx docs how to do that and got lost. So, not really "very simple".
Re: Benefits of using Sphinx documentation format
> Cc: g...@gcc.gnu.org, gcc-patches@gcc.gnu.org, jos...@codesourcery.com > From: Martin Liška > Date: Mon, 12 Jul 2021 16:34:11 +0200 > > > "Texinfo must go" is one possible conclusion from your description. > > But it isn't the only one. An alternative is "the Texinfo source of > > the GCC manual must be improved to fix this problem." And yes, this > > problem does have a solution in Texinfo. > > No, the alternative is more powerful output given by Texinfo, in particular > more modern HTML pages. Please see the response by Gavin: it sounds like at least some of that was resolved in Texinfo, sometimes long ago.
Re: Benefits of using Sphinx documentation format
> From: Jonathan Wakely > Date: Mon, 12 Jul 2021 15:05:11 +0100 > Cc: Martin Liška , > "g...@gcc.gnu.org" , gcc-patches > , > "Joseph S. Myers" > > To be clear, I give links to users frequently (several times a week, > every week, for decades) and prefer to give them a link to specific > options. Obviously I link to the online HTML docs rather than telling > them an 'info' command to run, because most people don't use info > pages or know how to navigate them. That means I can't provide decent > links, because the actual option name I'm trying to link to is always > off the top of the page. This is simply unacceptable IMHO. Texinfo > must go. "Texinfo must go" is one possible conclusion from your description. But it isn't the only one. An alternative is "the Texinfo source of the GCC manual must be improved to fix this problem." And yes, this problem does have a solution in Texinfo.
Re: Benefits of using Sphinx documentation format
> From: Jonathan Wakely > Date: Mon, 12 Jul 2021 14:53:44 +0100 > Cc: Martin Liška , > "g...@gcc.gnu.org" , gcc-patches > , > "Joseph S. Myers" > > For me, these items are enough justification to switch away from > texinfo, which produces crap HTML pages with crap anchors. If we want to have a serious discussion with useful conclusions, I suggest to avoid "loaded" terminology. I get it that you dislike the HTML produced by Texinfo, but without some examples of such bad HTML it is impossible to know what exactly do you dislike and why. > You can't find out the anchors without inspecting (and searching) > the HTML source. That's utterly stupid. I don't think I follow: find out the anchors with which means and for what purposes? > And even after you do that, the anchor > is at the wrong place: > https://gcc.gnu.org/onlinedocs/gcc/Overall-Options.html#index-c IME, the anchor is where you put it. If you show me the source of that HTMl, maybe we can have a more useful discussion of the issue. > As somebody who spends a lot of time helping users on the mailing > list, IRC, stackoverflow, and elsewhere, this "feature" of the texinfo > HTML has angered me for many years. As somebody who spends a lot of time helping users on every possible forum, and as someone who has wrote a lot of Texinfo, I don't understand what angers you. Please elaborate. > Yes, some people like texinfo, but some people also dislike it and > there are serious usability problems with the output. I support > replacing texinfo with anything that isn't texinfo. "Anything"? Even plain text? I hope not. See, such "arguments" don't help to have a useful discussion. > > 4) The need to learn yet another markup language. > > While this is not a problem for simple text, it does require a > > serious study of RST and Sphinx to use the more advanced features. > > This is a problem with texinfo too. Not for someone who already knows Texinfo. We are talking about switching away of it, so I'm thinking about people who contributed patches for the manual in the past. They already know Texinfo, at least to some extent, and some of them know it very well. > > 5) Lack of macros. > > AFAIK, only simple textual substitution is available, no macros > > with arguments. > > Is this a problem for GCC docs though? I don't know. It could be, even if it isn't now.
Re: Benefits of using Sphinx documentation format
> Cc: g...@gcc.gnu.org, gcc-patches@gcc.gnu.org, jos...@codesourcery.com > From: Martin Liška > Date: Mon, 12 Jul 2021 15:25:47 +0200 > > Let's make it a separate sub-thread where we can discuss motivation why > do I want moving to Sphinx format. Thanks for starting this discussion. > Benefits: > 1) modern looking HTML output (before: [1], after: [2]): > a) syntax highlighting for examples (code, shell commands, etc.) > b) precise anchors, the current Texinfo anchors are not displayed (start > with first line of an option) > c) one can easily copy a link to an anchor (displayed as ¶) > d) internal links are working, e.g. one can easily jump from listing of > options > e) left menu navigation provides better orientation in the manual > f) Sphinx provides internal search capability: [3] > 2) internal links are also provided in PDF version of the manual How is this different from Texinfo? > 3) some existing GCC manuals are already written in Sphinx (GNAT manuals and > libgccjit) > 4) support for various output formats, some people are interested in ePUB > format Texinfo likewise supports many output formats. Someone presented a very simple package to produce epub format from it. > 5) Sphinx is using RST which is quite minimal semantic markup language Is it more minimal than Texinfo? > 6) TOC is automatically generated - no need for manual navigation like seen > here: [5] That is not needed in Texinfo as well, since long ago. Nowadays, you just say @node Whatever and the rest is done automatically, as long as the manual's structure is a proper tree (which it normally is, I know of only one manual that is an exception). > Disadvantages: > > 1) info pages are currently missing Page description in TOC > 2) rich formatting is leading to extra wrapping in info output - beings > partially addresses in [4] > 3) one needs e.g. Emacs support for inline links (rendered as notes) 4) The need to learn yet another markup language. While this is not a problem for simple text, it does require a serious study of RST and Sphinx to use the more advanced features. 5) Lack of macros. AFAIK, only simple textual substitution is available, no macros with arguments.
Re: [PATCH] Port GCC documentation to Sphinx
> From: Richard Sandiford
> Cc: Eli Zaretskii , g...@gcc.gnu.org,
> gcc-patches@gcc.gnu.org, jos...@codesourcery.com
> Date: Mon, 05 Jul 2021 10:17:38 +0100
>
> Hans-Peter Nilsson writes:
> > I've read the discussion downthread, but I seem to miss (a recap
> > of) the benefits of moving to Sphinx. Maybe other have too and
> > it'd be a good idea to repeat them? Otherwise, the impression
> > is not so good, as all I see is bits here and there getting lost
> > in translation.
>
> Better cross-referencing is one big feature.
See below: the Info format has some features in addition to
cross-references that can make this a much smaller issue. HTML has
just the cross-references, so "when you are a hammer, every problem
looks like a nail".
> IMO this subthread has demonstrated why the limitations of info
> formatting have held back the amount of cross-referencing in the
> online html.
I disagree with this conclusion, see below.
> (And based on emperical evidence, I get the impression that far more
> people use the online html docs than the info docs.)
HTML browsers currently lack some features that make Info the format
of choice for me when I need to use the documentation efficiently.
The most important feature I miss in HTML browsers is the index
search. A good manual usually has extensive index (or indices) which
make it very easy to find a specific topic one is looking for,
i.e. use the manual as a reference (as opposed as a first-time
reading, when you read large portions of the manual in sequence).
Another important feature is regexp search across multiple sections
(with HTML you'd be forced to download the manual as a single large
file for that, and then you'll probably miss regexps).
Yet another feature which, when needed, is something to kill for, is
the "info apropos" command, which can search all the manuals on your
system and build a menu from the matching sections found in different
manuals. And there are a few more.
(Texinfo folks are working on JavaScript code to add some missing
capabilities to Web browsers, but that effort is not yet complete.)
> E.g. quoting from Richard's recent patch:
>
> @item -fmove-loop-stores
> @opindex fmove-loop-stores
> Enables the loop store motion pass in the GIMPLE loop optimizer. This
> moves invariant stores to after the end of the loop in exchange for
> carrying the stored value in a register across the iteration.
> Note for this option to have an effect @option{-ftree-loop-im} has to
> be enabled as well. Enabled at level @option{-O1} and higher, except
> for @option{-Og}.
>
> In the online docs, this will just be plain text. Anyone who doesn't
> know what -ftree-loop-im is will have to search for it manually.
First, even if there are no cross-references, manual search is not the
best way. It is much easier to use index-search:
i ftree TAB
will display a list of options that you could be after, and you can
simply choose from the list, or type a bit more until you have a
single match.
Moreover, adding cross-references is easy:
@item -fmove-loop-stores
@opindex fmove-loop-stores
Enables the loop store motion pass in the GIMPLE loop optimizer. This
moves invariant stores to after the end of the loop in exchange for
carrying the stored value in a register across the iteration.
Note for this option to have an effect @option{-ftree-loop-im}
(@pxref{Optimize Options, -ftree-loop-im})
^^
has be enabled as well. Enabled at level @option{-O1} and higher,
except for @option{-Og}.
If this looks like too much work, a simple Texinfo macro (two, if you
want an anchor where you point) will do.
> Adding the extra references to the html (and pdf) output but dropping
> them from the info sounds like a good compromise.
But that's not what happens. And besides, how would you decide which
cross-references to drop and which to retain in Info?
Re: [PATCH] Port GCC documentation to Sphinx
> Cc: Eli Zaretskii , g...@gcc.gnu.org, gcc-patches@gcc.gnu.org,
> jos...@codesourcery.com
> From: Martin Liška
> Date: Fri, 2 Jul 2021 11:40:06 +0200
>
> > It must
> > look sensible without that. In this case it seems that already the
> > generated .texinfo input to makeinfo is bad, where does the 'e' (or 'f')
> > come from? The original texinfo file simply contains:
>
> These are auto-numbered. Theoretically one can use the verbose anchor names:
>
> @anchor{demo cmdoption-Wshift-overflow3}@anchor{e}@anchor{demo
> cmdoption-wshift-overflow3}@anchor{f}
> @deffn {Option} @w{-}Wshift@w{-}overflow3=n, @w{-}Wshift@w{-}overflow3
>
> Default option value for @ref{e,,-Wshift-overflow3}.
>
> But these would lead to even longer '*note -Wshift-overflow3: demo
> cmdoption-wshift-overflow3' output.
While auto-numbering is a nice feature, the human-readable anchors
have an advantage of hinting on the topic to which the cross-reference
points.
Re: [PATCH] Port GCC documentation to Sphinx
> Cc: jos...@codesourcery.com, g...@gcc.gnu.org, gcc-patches@gcc.gnu.org > From: Martin Liška > Date: Fri, 2 Jul 2021 11:30:02 +0200 > > > So the purpose of having the comma there is to avoid having a period > > in the middle of a sentence, which is added by makeinfo (because the > > Info readers need that). Having a comma there may seem a bit > > redundant, but having a period will definitely look like a typo, if > > not confuse the heck out of the reader, especially if you want to use > > these inline cross-references so massively. > > Well, then it's a bug in Makeinfo. No, it isn't a bug in makeinfo. It's a (mis)feature of the Info format: a cross-reference needs to have a punctuation character after it, so that Info readers would know where's the end of the node/anchor name to which the cross-reference points. Info files are largely plain-ASCII files, so the Info readers need help in this case, and makeinfo produces what they need. > >> What type of conversions and style are going to change with conversion to > >> Sphinx? > > > > Anything that is different from the style conventions described in the > > Texinfo manual. We have many such conventions. > > Which is supposed to be here: > https://www.gnu.org/prep/standards/html_node/Documentation.html#Documentation > right? > > I've just the text. About the shortening of section names in a TOC. I > couldn't find > it in the GNU Documentation manual. No, there's also a lot of style guidelines in the Texinfo manual itself. Basically, the documentation of almost every Texinfo directive includes some style guidelines, and there are also sections which are pure guidelines, like the nodes "Conventions", "Node Names", "Structuring Command Types", and some others. > >> Again, please show up concrete examples. What you describe is very > >> theoretical. > > > > We've already seen one: the style of writing inline cross-references > > with the equivalent of @ref. We also saw another: the way you > > converted the menus. It is quite clear to me that there will be > > others. So I'm not sure why you need more evidence that this could be > > a real issue. > > As explained, @ref are generated by Makeinfo in a strange way. > About the menus, I was unable to find it.. See the node "Menu Parts" in the Texinfo manual. If you look at other GNU manuals, you will see that it is a de-facto standard to provide most menu items with short descriptions. > > But maybe all of this is intentional: maybe the GCC project > > consciously and deliberately decided to move away of the GNU > > documentation style and conventions, and replace them with whatever > > the Sphinx and RST conventions are? In that case, there's no reason > > for me to even mention these aspects. > > My intention is preserving status quo as much as possible. Well, but you definitely deviated from the status quo, and it sounds like you did that deliberately, without any discussion. > On the other hand, Sphinx provides quite some nice features why I wanted to > use it. Which features are those?
Re: [PATCH] Port GCC documentation to Sphinx
> Cc: jos...@codesourcery.com, g...@gcc.gnu.org, gcc-patches@gcc.gnu.org
> From: Martin Liška
> Date: Thu, 1 Jul 2021 18:04:24 +0200
>
> > Emacs doesn't hide the period. But there shouldn't be a period to
> > begin with, since it's the middle of a sentence. The correct way of
> > writing this in Texinfo is to have some punctuation: a comma or a
> > semi-colon, after the closing brace, like this:
> >
> >This is the warning level of @ref{e,,-Wshift-overflow3}, and …
>
> I don't see why we should put a comma after an option reference.
You explained it yourself later on:
> It's all related to Texinfo. Sphinx generates e.g.
> Enabled by @ref{7,,-Wall} and something else.
>
> as documented here:
> https://www.gnu.org/software/texinfo/manual/texinfo/html_node/_0040ref.html
>
> Then it ends with the following info output:
>
> Enabled by *note -Wall: 7. and something else.
>
> So the period is added by Texinfo. If I put comma after a reference, then
> the period is not added there.
^
So the purpose of having the comma there is to avoid having a period
in the middle of a sentence, which is added by makeinfo (because the
Info readers need that). Having a comma there may seem a bit
redundant, but having a period will definitely look like a typo, if
not confuse the heck out of the reader, especially if you want to use
these inline cross-references so massively.
> > I don't think the GCC manuals should necessarily be bound by the
> > Sphinx standards. Where those standards are sub-optimal, it is
> > perfectly okay for GCC (and other projects) to deviate. GCC and other
> > GNU manuals used a certain style and convention for decades, so
> > there's more than enough experience and tradition to build on.
>
> What type of conversions and style are going to change with conversion to
> Sphinx?
Anything that is different from the style conventions described in the
Texinfo manual. We have many such conventions.
> Do you see any of them worse than what we have now?
I didn't bother reading the Sphinx guidelines yet, and don't know when
(and if) I will have time for that. I do think the comparison should
be part of the job or moving to Sphinx.
> > I will no longer pursue this point, but let me just say that I
> > consider it a mistake to throw away all the experience collected using
> > Texinfo just because Sphinx folks have other traditions and
> > conventions. It might be throwing the baby with the bathwater.
> >
>
> Again, please show up concrete examples. What you describe is very
> theoretical.
We've already seen one: the style of writing inline cross-references
with the equivalent of @ref. We also saw another: the way you
converted the menus. It is quite clear to me that there will be
others. So I'm not sure why you need more evidence that this could be
a real issue.
But maybe all of this is intentional: maybe the GCC project
consciously and deliberately decided to move away of the GNU
documentation style and conventions, and replace them with whatever
the Sphinx and RST conventions are? In that case, there's no reason
for me to even mention these aspects.
Re: [PATCH] Port GCC documentation to Sphinx
> Cc: jos...@codesourcery.com, g...@gcc.gnu.org, gcc-patches@gcc.gnu.org
> From: Martin Liška
> Date: Thu, 1 Jul 2021 16:14:30 +0200
>
> >> If I understand the notes correct, the '.' should be also hidden by e.g.
> >> Emacs.
> >
> > No, it doesn't. The actual text in the Info file is:
> >
> > *note -std: f.‘=iso9899:1990’
> >
> > and the period after " f" isn't hidden. Where does that "f" come from
> > and what is its purpose here? can it be removed (together with the
> > period)?
>
> It's name of the anchor used for the @ref. The names are automatically
> generated
> by makeinfo. So there's an example:
>
> This is the warning level of @ref{e,,-Wshift-overflow3} and …
>
> becomes in info:
> This is the warning level of *note -Wshift-overflow3: e. and …
>
> I can ask the question at Sphinx, the Emacs script should hide that.
Emacs doesn't hide the period. But there shouldn't be a period to
begin with, since it's the middle of a sentence. The correct way of
writing this in Texinfo is to have some punctuation: a comma or a
semi-colon, after the closing brace, like this:
This is the warning level of @ref{e,,-Wshift-overflow3}, and …
Does Sphinx somehow generate the period if there's no comma, or does
it do it unconditionally, i.e. even if there is a punctuation after
the closing brace?
> > This actually raises a more general issue with this Sphinx porting
> > initiative: what will be the canonical style guide for maintaining the
> > GCC manual in Sphinx, or more generally for writing GNU manuals in
> > Sphinx? For Texinfo, we have the Texinfo manual, which both documents
> > the language and provides style guidelines for how to use Texinfo for
> > producing good manuals. Contributors to GNU manuals are using those
> > guidelines for many years. Is there, or will there be, an equivalent
> > style guide for Sphinx? If not, how will the future contributors to
> > the GCC manuals know what are the writing and style conventions?
>
> No, I'm not planning any extra style guide. We will you standard Sphinx RST
> manual and one can find many tutorials about how to do it.
Are you sure everything there is good for our manuals? Did you
compare the style conventions there with what we have in the Texinfo
manual?
Moreover, this means people who contribute to other manuals will now
have to learn two different styles, no? And that's in addition to
learning one more language.
> > That is why I recommended to discuss this on the Texinfo list: that's
> > the place where such guidelines are discussed, and where we have
> > experts who understand the effects and consequences of using this or
> > that style. The current style in GNU manuals is to have the menus as
> > we see them in the existing GCC manuals: with a short description.
> > Maybe there are good reasons to deviate from that style, but
> > shouldn't this be at least presented and discussed, before the
> > decision is made? GCC developers are not the only ones who will be
> > reading the future GCC manuals.
> >
>
> That seems to me a subtle adjustment and it's standard way how people generate
> TOC in Sphinx. See e.g. the Linux kernel documentation:
> https://www.kernel.org/doc/html/latest/
I don't think the GCC manuals should necessarily be bound by the
Sphinx standards. Where those standards are sub-optimal, it is
perfectly okay for GCC (and other projects) to deviate. GCC and other
GNU manuals used a certain style and convention for decades, so
there's more than enough experience and tradition to build on.
I will no longer pursue this point, but let me just say that I
consider it a mistake to throw away all the experience collected using
Texinfo just because Sphinx folks have other traditions and
conventions. It might be throwing the baby with the bathwater.
Re: [PATCH] Port GCC documentation to Sphinx
> Cc: jos...@codesourcery.com, g...@gcc.gnu.org, gcc-patches@gcc.gnu.org
> From: Martin Liška
> Date: Thu, 1 Jul 2021 14:44:10 +0200
>
> > It helps some, but not all of the issues disappear. For example,
> > stuff like this is still hard to read:
> >
> >To select this standard in GCC, use one of the options -ansi
> > -
> >-std.‘=c90’ or -std.‘=iso9899:1990’
> >
>
> If I understand the notes correct, the '.' should be also hidden by e.g.
> Emacs.
No, it doesn't. The actual text in the Info file is:
*note -std: f.‘=iso9899:1990’
and the period after " f" isn't hidden. Where does that "f" come from
and what is its purpose here? can it be removed (together with the
period)?
> About ‘=iso9899:1990’, yes, it's a :samp: and how it's wrapper by Sphinx by
> default.
Why is it a separate :samp:? IMO, the correct markup is to make the
entire string -std=iso9899:1990 have the same typeface. In Texinfo,
I'd use
@option{-std=iso9899:1990}
> >> We can adjust 'emph' formatting to nil, what do you think?
> >
> > Something like that, yes. But the problem is: how will you format it
> > instead? The known alternatives, _foo_ and *foo* both use punctuation
> > characters, which will get in the way similarly to the quotes. Can
> > you format those in caps, like makeinfo does?
>
> You are fully right, info is very simple format and it uses wrapping for the
> formatting
> purpose (by default * and _). So, I don't have any elegant solution.
Well, it sounds from another mail that you did find a solution: to
up-case the string in @var.
> >>> Note also that nodes are now called by the same name as the section,
> >>> which means node names generally got much longer. Is that really a
> >>> good idea?
> >>
> >> Well, I intentionally removed these and used simple TOC tree links
> >> which take display text for a section title.
> >
> > I would suggest to discuss these decisions first, perhaps on the
> > Texinfo mailing list? I'm accustomed to these short descriptions, but
> > I'm not sure how important they are for others.
>
> Well, it was decision done during the transition of texinfo files into Sphinx.
> I don't see why it should be discussed in Texinfo community.
This actually raises a more general issue with this Sphinx porting
initiative: what will be the canonical style guide for maintaining the
GCC manual in Sphinx, or more generally for writing GNU manuals in
Sphinx? For Texinfo, we have the Texinfo manual, which both documents
the language and provides style guidelines for how to use Texinfo for
producing good manuals. Contributors to GNU manuals are using those
guidelines for many years. Is there, or will there be, an equivalent
style guide for Sphinx? If not, how will the future contributors to
the GCC manuals know what are the writing and style conventions?
That is why I recommended to discuss this on the Texinfo list: that's
the place where such guidelines are discussed, and where we have
experts who understand the effects and consequences of using this or
that style. The current style in GNU manuals is to have the menus as
we see them in the existing GCC manuals: with a short description.
Maybe there are good reasons to deviate from that style, but
shouldn't this be at least presented and discussed, before the
decision is made? GCC developers are not the only ones who will be
reading the future GCC manuals.
Re: [PATCH] Port GCC documentation to Sphinx
> Cc: jos...@codesourcery.com, g...@gcc.gnu.org, gcc-patches@gcc.gnu.org
> From: Martin Liška
> Date: Wed, 30 Jun 2021 16:04:32 +0200
>
> > Thanks, but does that mean @var will no longer stand out in the
> > produced Info format? That'd be sub-optimal, I think, because a clear
> > reference to a meta-syntactic variable will be lost.
>
> Yes. An alternative approach for:
> Show :samp:`Samp with a {variable}.`
>
> can be using @var{variable}, resulting with the following info output:
> Show ‘Samp with a VARIABLE.’
>
> Does it seem reasonable?
Yes, the latter sounds reasonable.
Thanks.
Re: [PATCH] Port GCC documentation to Sphinx
> Cc: jos...@codesourcery.com, g...@gcc.gnu.org, gcc-patches@gcc.gnu.org > From: Martin Liška > Date: Wed, 30 Jun 2021 15:28:40 +0200 > > >‘@`file'’ > > > > Read command-line options from ‘`file'’. The options read are > > inserted in place of the original ‘@`file'’ option. If ‘`file'’ > > does not exist, or cannot be read, then the option will be treated > > literally, and not removed. > > For this one, I've just created the following pull request: > https://github.com/sphinx-doc/sphinx/pull/9391 Thanks, but does that mean @var will no longer stand out in the produced Info format? That'd be sub-optimal, I think, because a clear reference to a meta-syntactic variable will be lost.
Re: [PATCH] Port GCC documentation to Sphinx
> Cc: jos...@codesourcery.com, g...@gcc.gnu.org, gcc-patches@gcc.gnu.org
> From: Martin Liška
> Date: Wed, 30 Jun 2021 12:11:03 +0200
>
> > (Admittedly, Emacs by default hides some of the text of a
> > cross-reference, but not hiding them in this case produces an even
> > less legible text.)
>
> If I'm correct, it's exactly what's documented in Sphinx FAQ here:
> https://www.sphinx-doc.org/en/master/faq.html#displaying-links
>
> and there's a suggested Emacs code snippet that should help with links.
> Does it help?
It helps some, but not all of the issues disappear. For example,
stuff like this is still hard to read:
To select this standard in GCC, use one of the options -ansi
-
-std.‘=c90’ or -std.‘=iso9899:1990’
The quotes around the option values don't help.
Also, using the method proposed by Sphinx FAQ would need a change in
Emacs, which will take time to propagate. So my suggestion is to
minimize the use of such "inline" hyperlinks.
> >‘@`file'’
> >
> > Read command-line options from ‘`file'’. The options read are
> > inserted in place of the original ‘@`file'’ option. If ‘`file'’
> > does not exist, or cannot be read, then the option will be treated
> > literally, and not removed.
>
> I can confirm that, so e.g.
> Show :samp:`Samp with a {variable}.`
>
> is transformed into:
> Show @code{Samp with a @emph{variable}.}
>
> Default info formatting is selected as:
>
> @definfoenclose strong,`,'
> @definfoenclose emph,`,'
>
> We can adjust 'emph' formatting to nil, what do you think?
Something like that, yes. But the problem is: how will you format it
instead? The known alternatives, _foo_ and *foo* both use punctuation
characters, which will get in the way similarly to the quotes. Can
you format those in caps, like makeinfo does?
> > 4. Menus lost the short descriptions of the sub-sections. Example:
> >
> >* Designated Initializers
> >* Case Ranges
> >* Cast to a Union Type
> >* Mixed Declarations, Labels and Code
> >* Declaring Attributes of Functions
> >
> > vs
> >
> >* Designated Inits::Labeling elements of initializers.
> >* Case Ranges:: 'case 1 ... 9' and such.
> >* Cast to Union:: Casting to union type from any member of the
> > union.
> >* Mixed Declarations:: Mixing declarations and code.
> >* Function Attributes:: Declaring that functions have no side effects,
> > or that they can never return.
> >
> > Looks like some bug to me.
> >
> > Note also that nodes are now called by the same name as the section,
> > which means node names generally got much longer. Is that really a
> > good idea?
>
> Well, I intentionally removed these and used simple TOC tree links
> which take display text for a section title.
I would suggest to discuss these decisions first, perhaps on the
Texinfo mailing list? I'm accustomed to these short descriptions, but
I'm not sure how important they are for others.
Re: [PATCH] Port GCC documentation to Sphinx
> Date: Tue, 29 Jun 2021 19:57:11 +0300 > From: Eli Zaretskii via Gcc > Cc: g...@gcc.gnu.org, gcc-patches@gcc.gnu.org, jos...@codesourcery.com > > Or how about this: > > `Overall Options' > >See Options Controlling the Kind of Output. > >*note -c. *note -S. *note -E. *note -o. ‘`file'’ >*note -dumpbase. ‘`dumpbase'’ *note -dumpbase-ext. >‘`auxdropsuf'’ *note -dumpdir. ‘`dumppfx'’ ‘-x’ ‘`language'’ >*note -v. *note -###. *note –help.‘[=`class'[,...]]’ >*note –target-help. *note –version. *note -pass-exit-codes >. *note -pipe. *note -specs.‘=`file'’ *note -wrapper >.‘@`file'’ *note -ffile-prefix-map.‘=`old'=`new'’ *note >-fplugin.‘=`file'’ ‘-fplugin-arg-’‘`name'=`arg'’ >‘-fdump-ada-spec’‘[-`slim']’ *note -fada-spec-parent.‘=`unit'’ >*note -fdump-go-spec.‘=`file'’ I see that when I copied text into the mail, the "see" that Emacs displays got replaced by "*note" (which is what actually appears in the Info file). So if you want to understand my references to the ubiquitous "see", imagine that each "*note" is displayed as "see". Apologies for any confusion.
Re: [PATCH] Port GCC documentation to Sphinx
> From: Martin Liška
> Date: Tue, 29 Jun 2021 12:09:23 +0200
> Cc: GCC Development , gcc-patches@gcc.gnu.org
>
> On 6/28/21 5:33 PM, Joseph Myers wrote:
> > Are formatted manuals (HTML, PDF, man, info) corresponding to this patch
> > version also available for review?
>
> I've just uploaded them here:
> https://splichal.eu/gccsphinx-final/
Thanks.
I'm an Info junkie, so I grabbed gcc.info from there and skimmed
through it. Please allow me a few unsolicited comments:
1. It sounds like Sphinx is heavily biased towards HTML format, and as
result uglifies the Info format?
For example, many cross-references (AFAIU introduced as part of
migration to Sphinx) make the text illegible in Emacs. Example:
This standard, in both its forms, is commonly known as `C89', or
occasionally as `C90', from the dates of ratification. To select this
standard in GCC, use one of the options *note -ansi *note -std
.‘=c90’ or *note -std.‘=iso9899:1990’; to obtain all the diagnostics
required by the standard, you should also specify *note -pedantic.
(or *note -pedantic-errors. if you want them to be errors rather
than warnings). See *note Options Controlling C Dialect.
[...]
An amendment to the 1990 standard was published in 1995. This amendment
added digraphs and ‘__STDC_VERSION__’ to the language, but otherwise
concerned the library. This amendment is commonly known as `AMD1'; the
amended standard is sometimes known as `C94' or `C95'. To select this
standard in GCC, use the option *note -std.‘=iso9899:199409’ (with,
as for other standard versions, *note -pedantic. to receive all
required diagnostics).
Or how about this:
`Overall Options'
See Options Controlling the Kind of Output.
*note -c. *note -S. *note -E. *note -o. ‘`file'’
*note -dumpbase. ‘`dumpbase'’ *note -dumpbase-ext.
‘`auxdropsuf'’ *note -dumpdir. ‘`dumppfx'’ ‘-x’ ‘`language'’
*note -v. *note -###. *note –help.‘[=`class'[,...]]’
*note –target-help. *note –version. *note -pass-exit-codes
. *note -pipe. *note -specs.‘=`file'’ *note -wrapper
.‘@`file'’ *note -ffile-prefix-map.‘=`old'=`new'’ *note
-fplugin.‘=`file'’ ‘-fplugin-arg-’‘`name'=`arg'’
‘-fdump-ada-spec’‘[-`slim']’ *note -fada-spec-parent.‘=`unit'’
*note -fdump-go-spec.‘=`file'’
In the first line, the emphasis became quotes, which sounds sub-optimal.
In the second line, the hyperlink was lost.
And the rest is not really readable.
Compare this with the original:
_Overall Options_
*Note Options Controlling the Kind of Output.
-c -S -E -o FILE -x LANGUAGE
-v -### --help[=CLASS[,...]] --target-help --version
-pass-exit-codes -pipe -specs=FILE -wrapper
@FILE -ffile-prefix-map=OLD=NEW
-fplugin=FILE -fplugin-arg-NAME=ARG
-fdump-ada-spec[-slim] -fada-spec-parent=UNIT -fdump-go-spec=FILE
(Admittedly, Emacs by default hides some of the text of a
cross-reference, but not hiding them in this case produces an even
less legible text.)
In general, it is a well-known rule that Texinfo documentation should
NOT use @ref{foo} as if @ref will disappear without a trace, leaving
just the hyperlink to 'foo'. Looks like the rewritten manual uses
that a lot.
This "see" consistently gets in the way throughout the entire manual.
A few more examples:
-- Option: -flocal-ivars
Default option value for *note -fno-local-ivars.
...
For example *note -std.‘=gnu90 -Wpedantic’ warns about C++ style
‘//’ comments, while *note -std.‘=gnu99 -Wpedantic’ does not.
...
If this option is not provided but *note -Wabi.‘=`n'’ is, that
version is used for compatibility aliases.
...
Below *note -std.‘=c++20’, *note -fconcepts. enables
support for the C++ Extensions for Concepts Technical
Specification, ISO 19217 (2015).
...
gcov [ *note -v. | *note –version. ] [ ‘-h’ | *note –help. ]
2. The translation of @var produces double-quoting in Info, here's an
example:
The usual way to run GCC is to run the executable called ‘gcc’, or
‘`machine'-gcc’ when cross-compiling, or ‘`machine'-gcc-`version'’ to
run a specific version of GCC.
vs, the old
The usual way to run GCC is to run the executable called 'gcc', or
'MACHINE-gcc' when cross-compiling, or 'MACHINE-gcc-VERSION' to run a
specific version of GCC.
I think the new variant is less readable and more confusing, because
it isn't clear whether the quotes are part of the text. Here's an
extreme example:
‘@`file'’
Read command-line options from ‘`file'’. The options read are
inserted in place of the original ‘@`file'’ option. If ‘`file'’
does not exist, or cannot be read, then the option will be treated
literally, and not removed.
3. Some cross-references lost the hyperlinks:
See option-index, for an index to GCC’s options.
("option-index" was a hyperlink
Re: Compilation error in simple-object-elf.c
> Cc: d...@redhat.com, gcc-patches@gcc.gnu.org, gdb-patc...@sourceware.org > From: Pedro Alves > Date: Thu, 19 Jul 2018 14:41:13 +0100 > > On 07/19/2018 02:06 PM, Eli Zaretskii wrote: > >> From: Richard Biener > >> Date: Thu, 19 Jul 2018 10:46:01 +0200 > >> Cc: DJ Delorie , GCC Patches , > >> gdb-patc...@sourceware.org > >> > >>> *err = ENOTSUP; > >>>^~~ > >>> ./simple-object-elf.c:1284:14: note: each undeclared identifier is > >>> reported only once for each function it appears in > >>> > >>> Suggested fix: > >> > >> Works for me, thus OK. I'm going to check it in to make 8.2. > > > > Thanks. > > > > Joel/Pedro, is this okay for GDB's copy of libiberty, master and > > branch? Thanks, done.
Re: Compilation error in simple-object-elf.c
> From: Richard Biener > Date: Thu, 19 Jul 2018 10:46:01 +0200 > Cc: DJ Delorie , GCC Patches , > gdb-patc...@sourceware.org > > > *err = ENOTSUP; > >^~~ > > ./simple-object-elf.c:1284:14: note: each undeclared identifier is > > reported only once for each function it appears in > > > > Suggested fix: > > Works for me, thus OK. I'm going to check it in to make 8.2. Thanks. Joel/Pedro, is this okay for GDB's copy of libiberty, master and branch?
Compilation error in simple-object-elf.c
Hi, I've built the pretest of GDB 8.2 with MinGW today, and bumped into a compilation error in libiberty: if [ x"" != x ]; then \ gcc -c -DHAVE_CONFIG_H -O2 -gdwarf-4 -g3 -D__USE_MINGW_ACCESS -I. -I./../include -W -Wall -Wwrite-strings -Wc++-compat -Wstrict-prototypes -pedantic -D_GNU_SOURCE ./simple-object-elf.c -o noasan/simple-object-elf.o; \ else true; fi gcc -c -DHAVE_CONFIG_H -O2 -gdwarf-4 -g3 -D__USE_MINGW_ACCESS -I. -I./../include -W -Wall -Wwrite-strings -Wc++-compat -Wstrict-prototypes -pedantic -D_GNU_SOURCE ./simple-object-elf.c -o simple-object-elf.o ./simple-object-elf.c: In function 'simple_object_elf_copy_lto_debug_sections': ./simple-object-elf.c:1284:14: error: 'ENOTSUP' undeclared (first use in this function) *err = ENOTSUP; ^~~ ./simple-object-elf.c:1284:14: note: each undeclared identifier is reported only once for each function it appears in Suggested fix: 2018-07-18 Eli Zaretskii * libiberty/simple-object-elf.c (ENOTSUP): If not defined by errno.h, redirect to ENOSYS. --- libiberty/simple-object-elf.c~0 2018-07-04 18:41:59.0 +0300 +++ libiberty/simple-object-elf.c 2018-07-18 18:19:39.286654700 +0300 @@ -22,6 +22,10 @@ Boston, MA 02110-1301, USA. */ #include "simple-object.h" #include +/* mingw.org's MinGW doesn't have ENOTSUP. */ +#ifndef ENOTSUP +# define ENOTSUP ENOSYS +#endif #include #ifdef HAVE_STDLIB_H
Re: Compilation warning in simple-object-xcoff.c
> From: Ian Lance Taylor > Date: Sat, 20 Jan 2018 21:01:09 -0800 > Cc: DJ Delorie , gcc-patches , > gdb-patches > > On Sat, Jan 20, 2018 at 4:47 AM, Eli Zaretskii wrote: > >> Date: Thu, 18 Jan 2018 05:25:20 +0200 > >> From: Eli Zaretskii > >> CC: sch...@linux-m68k.org, gcc-patches@gcc.gnu.org, > >> gdb-patc...@sourceware.org > >> > >> > From: DJ Delorie > >> > Cc: sch...@linux-m68k.org, gcc-patches@gcc.gnu.org, > >> > gdb-patc...@sourceware.org > >> > Date: Wed, 17 Jan 2018 15:47:49 -0500 > >> > > >> > Eli Zaretskii writes: > >> > > >> > > DJ, would the following semi-kludgey workaround be acceptable? > >> > > >> > It would be no worse than what we have now, if the only purpose is to > >> > avoid a warning. > >> > > >> > Ideally, we would check to see if we're discarding non-zero values from > >> > that offset, and not call the callback with known bogus data. I suppose > >> > the usefulness of that depends on how often you'll encounter 4Gb+ xcoff64 > >> > files on mingw32 ? > >> > >> The answer to that question is "never", AFAIU. > > > > So can the patch I proposed be applied, please? > > I committed the patch. Thanks, Ian!
Re: Compilation warning in simple-object-xcoff.c
> Date: Thu, 18 Jan 2018 05:25:20 +0200 > From: Eli Zaretskii > CC: sch...@linux-m68k.org, gcc-patches@gcc.gnu.org, > gdb-patc...@sourceware.org > > > From: DJ Delorie > > Cc: sch...@linux-m68k.org, gcc-patches@gcc.gnu.org, > > gdb-patc...@sourceware.org > > Date: Wed, 17 Jan 2018 15:47:49 -0500 > > > > Eli Zaretskii writes: > > > > > DJ, would the following semi-kludgey workaround be acceptable? > > > > It would be no worse than what we have now, if the only purpose is to > > avoid a warning. > > > > Ideally, we would check to see if we're discarding non-zero values from > > that offset, and not call the callback with known bogus data. I suppose > > the usefulness of that depends on how often you'll encounter 4Gb+ xcoff64 > > files on mingw32 ? > > The answer to that question is "never", AFAIU. So can the patch I proposed be applied, please? TIA
Re: Compilation warning in simple-object-xcoff.c
> From: DJ Delorie > Cc: sch...@linux-m68k.org, gcc-patches@gcc.gnu.org, gdb-patc...@sourceware.org > Date: Wed, 17 Jan 2018 15:47:49 -0500 > > Eli Zaretskii writes: > > > DJ, would the following semi-kludgey workaround be acceptable? > > It would be no worse than what we have now, if the only purpose is to > avoid a warning. > > Ideally, we would check to see if we're discarding non-zero values from > that offset, and not call the callback with known bogus data. I suppose > the usefulness of that depends on how often you'll encounter 4Gb+ xcoff64 > files on mingw32 ? The answer to that question is "never", AFAIU.
Re: Compilation warning in simple-object-xcoff.c
> From: Andreas Schwab
> Cc: Eli Zaretskii , gcc-patches@gcc.gnu.org,
> gdb-patc...@sourceware.org
> Date: Tue, 16 Jan 2018 23:00:55 +0100
>
> On Jan 16 2018, DJ Delorie wrote:
>
> > And it's not the host's bit size that counts; there are usually ways to
> > get 64-bit file operations on 32-bit hosts.
>
> If ACX_LARGEFILE doesn't succeed in enabling those 64-bit file
> operations (thus making off_t a 64-bit type) then you are out of luck
> (or AC_SYS_LARGEFILE doesn't support your host yet).
Yes, AC_SYS_LARGEFILE doesn't support MinGW.
DJ, would the following semi-kludgey workaround be acceptable?
--- libiberty/simple-object-xcoff.c~0 2018-01-12 05:31:04.0 +0200
+++ libiberty/simple-object-xcoff.c 2018-01-17 12:21:08.496186000 +0200
@@ -596,13 +596,19 @@ simple_object_xcoff_find_sections (simpl
aux = (unsigned char *) auxent;
if (u64)
{
+ /* Use an intermediate 64-bit type to avoid
+compilation warning about 32-bit shift below on
+hosts with 32-bit off_t which aren't supported by
+AC_SYS_LARGEFILE. */
+ ulong_type x_scnlen64;
+
if ((auxent->u.xcoff64.x_csect.x_smtyp & 0x7) != XTY_SD
|| auxent->u.xcoff64.x_csect.x_smclas != XMC_XO)
continue;
- x_scnlen = fetch_32 (aux + offsetof (union external_auxent,
-
u.xcoff64.x_csect.x_scnlen_hi));
- x_scnlen = x_scnlen << 32
+ x_scnlen64 = fetch_32 (aux + offsetof (union external_auxent,
+
u.xcoff64.x_csect.x_scnlen_hi));
+ x_scnlen = x_scnlen64 << 32
| fetch_32 (aux + offsetof (union external_auxent,
u.xcoff64.x_csect.x_scnlen_lo));
}
Re: Compilation warning in simple-object-xcoff.c
> From: DJ Delorie > Cc: gcc-patches@gcc.gnu.org, gdb-patc...@sourceware.org > Date: Tue, 16 Jan 2018 13:00:48 -0500 > > > I think that warning is valid - the host has a 32-bit limit to file > sizes (off_t) but it's trying to read a 64-bit offset (in that clause). > It's warning you that you won't be able to handle files as large as the > field implies. If 32-bit off_t cannot handle this, then perhaps this file (or that function) should not be compiled for a 32-bit host?
Compilation warning in simple-object-xcoff.c
Compiling GDB 8.0.91 with mingw.org's MinGW GCC 6.0.3 produces this warning in libiberty: gcc -c -DHAVE_CONFIG_H -O2 -gdwarf-4 -g3 -D__USE_MINGW_ACCESS -I. -I./../include -W -Wall -Wwrite-strings -Wc++-compat -Wstrict-prototypes -pedantic -D_GNU_SOURCE ./simple-object-xcoff.c -o simple-object-xcoff.o ./simple-object-xcoff.c: In function 'simple_object_xcoff_find_sections': ./simple-object-xcoff.c:605:25: warning: left shift count >= width of type [-Wshift-count-overflow] x_scnlen = x_scnlen << 32 ^~ And indeed x_scnlen is declared as a 32-bit data type off_t. I'm willing to test patches if needed. Thanks.
Re: MinGW compilation warnings in libiberty's xstrndup.c
> From: DJ Delorie > Cc: gcc-patches@gcc.gnu.org, gdb-patc...@sourceware.org > Date: Wed, 31 May 2017 00:17:14 -0400 > > > Eli Zaretskii writes: > > Seems to work fine, thanks. > > Checked into gcc trunk then :-) Thanks!
Re: MinGW compilation warnings in libiberty's xstrndup.c
> From: DJ Delorie > Cc: gcc-patches@gcc.gnu.org, gdb-patc...@sourceware.org > Date: Fri, 26 May 2017 17:34:12 -0400 > > > Please try this patch: Seems to work fine, thanks.
Re: MinGW compilation warnings in libiberty's waitpid.c
> Date: Fri, 26 May 2017 09:30:53 -0700 > From: Joel Brobecker > Cc: DJ Delorie , gcc-patches@gcc.gnu.org, > gdb-patc...@sourceware.org > > Normally, I'd expect someone pushing to GCC's libibert to also > update our repo accordingly. However, it's easy to forget so, > if you notice a change that was not propagated to us, we just > cherry-pick those changes so as to make sure our copy is up > to date with GCC's. We also see the occasional "resync libiberty" > commits which act as a failsafe, but I don't think we should wait > for one of those. What can I do to expedite the process? This currently holds the 8.0 release, and I'm uneasy to be the culprit. > Should those ones be pushed to the gdb-8.0-branch as well? Yes.
Re: MinGW compilation warnings in libiberty's waitpid.c
> From: DJ Delorie > Cc: gcc-patches@gcc.gnu.org, gdb-patc...@sourceware.org > Date: Wed, 24 May 2017 17:36:16 -0400 > > Thanks. Committed! Thanks. Is the environ thing also fixed? Joel/Pedro, how should I go about making sure these changes are in the GDB copy of libiberty?
Re: MinGW compilation warnings in libiberty's waitpid.c
> From: DJ Delorie > Cc: gcc-patches@gcc.gnu.org, gdb-patc...@sourceware.org > Date: Tue, 23 May 2017 22:26:22 -0400 > > > Is there anything else I need to do about this part to get it solved > > in the upstream repository? > > A ChangeLog entry would be nice, so I don't have to write one ;-) Sure: 2017-05-24 Eli Zaretskii * libiberty/waitpid.c (wait) [__MINGW32__]: Define as a macro that calls _cwait, so that this function works on MinGW.
Re: MinGW compilation warnings in libiberty's waitpid.c
> From: DJ Delorie > Cc: gcc-patches@gcc.gnu.org, gdb-patc...@sourceware.org > Date: Tue, 23 May 2017 15:37:32 -0400 > > > Eli Zaretskii writes: > > Instead of making waitpid an always-failing stub on MinGW, wouldn't it > > be better to make it work on MinGW? Like this: > > That's up to you, if it's target-specific. Then I prefer this solution. > What about mingw64? It will be covered as well, because it defines __MINGW32__, and because _cwait is in the MS runtime, so available to MinGW64, too. Is there anything else I need to do about this part to get it solved in the upstream repository? Thanks.
Re: MinGW compilation warnings in libiberty's waitpid.c
> From: DJ Delorie
> Cc: gcc-patches@gcc.gnu.org, gdb-patc...@sourceware.org
> Date: Mon, 22 May 2017 15:38:40 -0400
>
> Since (or "if") nobody will (should) use waitpid() on mingw anyway, and
> since libiberty really wants to include waitpid.o, how difficult would
> it be to use some #ifdefs to have waitpid() just return an error on
> mingw? That at least gets past the mingw build problem.
Instead of making waitpid an always-failing stub on MinGW, wouldn't it
be better to make it work on MinGW? Like this:
--- libiberty/waitpid.c~0 2016-08-01 18:50:21.0 +0300
+++ libiberty/waitpid.c 2017-05-23 21:19:34.302415000 +0300
@@ -23,6 +23,11 @@ does the return value. The third argume
#include
#endif
+#ifdef __MINGW32__
+#include
+#define wait(s) _cwait(s,pid,_WAIT_CHILD)
+#endif
+
pid_t
waitpid (pid_t pid, int *stat_loc, int options ATTRIBUTE_UNUSED)
{
Re: MinGW compilation warnings in libiberty's waitpid.c
> From: DJ Delorie > Cc: gcc-patches@gcc.gnu.org, gdb-patc...@sourceware.org > Date: Fri, 19 May 2017 21:28:25 -0400 > > > Please try this patch, since my mingw environment is old: > > Index: libiberty/ChangeLog > === > --- libiberty/ChangeLog (revision 248307) > +++ libiberty/ChangeLog (working copy) > @@ -1,3 +1,7 @@ > +2017-05-19 Eli Zaretskii > + > + * configure.ac (*-*-mingw*): Don't build waitpid.c. > + > 2017-05-02 Iain Buclaw > > * d-demangle.c (dlang_hexdigit): New function. > Index: libiberty/configure.ac > === > --- libiberty/configure.ac (revision 248307) > +++ libiberty/configure.ac (working copy) > @@ -493,7 +493,6 @@ > AC_LIBOBJ([strnlen]) > AC_LIBOBJ([strverscmp]) > AC_LIBOBJ([vasprintf]) > -AC_LIBOBJ([waitpid]) > > for f in $funcs; do >case "$f" in > Hmm... no, this doesn't solve the problem. The expansion of AC_LIBOBJ for waitpid is gone from the configure script, but the value of LIBOBJS in libiberty/Makefile still includes waitpid.o. What else is related to this? One caveat: I needed to hack config/override.m4 to allow me to run autoconf 2.69 I have installed, because otherwise it insists on autoconf 2.64 which I don't have. I hope this isn't the reason for the incomplete solution.
Re: MinGW compilation warnings in libiberty's include/environ.h
> From: DJ Delorie > Cc: pal...@redhat.com, gcc-patches@gcc.gnu.org, gdb-patc...@sourceware.org > Date: Sat, 20 May 2017 02:16:14 -0400 > > Eli Zaretskii writes: > > My problem is, I don't have a GCC repository, so doing the above means > > In this case, a gdb repo would have sufficed. Thanks, I didn't know that.
Re: MinGW compilation warnings in libiberty's include/environ.h
> From: DJ Delorie > Cc: gcc-patches@gcc.gnu.org, gdb-patc...@sourceware.org > Date: Fri, 19 May 2017 21:25:56 -0400 > > Fix committed. Thanks! Pedro, how do we make this propagated to the GDB repository? > As Pedro noted, the correct way to request a change is > to make the change in your local checked out repo, and run "svn diff" > (or "git diff"). My problem is, I don't have a GCC repository, so doing the above means checking it out, which takes a _very_ long time here. And the patch I was thinking about is not the one you eventually committed anyway, which is to be expected, since I have no idea about the various dependencies between the projects that use the common configure infrastructure and about how things are done to keep all the user projects in sync. IOW, not everyone who reports a problem can necessarily provide a patch. The fact that you know too much about my abilities in other projects doesn't (or shouldn't) change that ;-) > The easier you can make it for the maintainer (me!), the more likely > your patch will get handled quickly. I know. I was trying to make it as easy as I possibly could, given the level of my knowledge in this area.
Re: MinGW compilation warnings in libiberty's include/environ.h
> Cc: gdb-patc...@sourceware.org > From: Pedro Alves > Date: Fri, 19 May 2017 16:51:30 +0100 > > So again there's a system header that defines the symbol > but for some reason libiberty still wants to declare/define > it is if it weren't? Yes. AFAICS, libiberty's configure script doesn't check the declaration, it only probes the setenv function itself. You can see that the cpp directives around the environ declarations are OS-dependent rather than based on autoconf tests. > That sounds to me like the root issue that should be fixed, > so that these fallback definitions don't come into into play at all. > I.e., why isn't HAVE_ENVIRON_DECL defined on mingw when > setenv.o is built? Sounds like a decl check is missing > in configure.ac. Most probably, yes.
Re: MinGW compilation warnings in libiberty's waitpid.c
> Cc: gdb-patc...@sourceware.org > From: Pedro Alves > Date: Fri, 19 May 2017 16:36:46 +0100 > > So I wonder whether we could just unconditionally remove the waitpid > replacement instead. That's probably the best path forward.
Re: MinGW compilation warnings in libiberty's xstrndup.c
> Cc: gdb-patc...@sourceware.org, Thomas Schwinge > From: Pedro Alves > Date: Fri, 19 May 2017 16:22:55 +0100 > > But then, xstrndup.c has at the top: > > #ifdef HAVE_STRING_H > #include > #else > # ifdef HAVE_STRINGS_H > # include > # endif > #endif > > So I would expect your build to pick the strnlen declaration from > one of the string.h or strings.h mingw headers. Why didn't it? Because MinGW doesn't have it, not unless you build a program that will require one of the newer versions of the Windows C runtime library. That's why libiberty's strnlen is being compiled in the MinGW build in the first place. Specifically, the MinGW headers do provide a prototype for strnlen if the program defines __MSVCRT_VERSION__ to be a high enough version, or defines _POSIX_C_SOURCE >= 200809L, but none of these is set by default, and is not a good idea, as explained above, for a program that needs to run on a wide variety of OS versions. IOW, libiberty shouldn't rely on the system headers to provide a strnlen prototype when libiberty's strnlen is being included in the library as a replacement.
MinGW compilation warnings in libiberty's xstrndup.c
When compiling libiberty (as part of GDB) with MinGW on MS-Windows, I see the following warning: gcc -c -DHAVE_CONFIG_H -O2 -gdwarf-4 -g3 -D__USE_MINGW_ACCESS -I. -I./../include -W -Wall -Wwrite-strings -Wc++-compat -Wstrict-prototypes -pedantic -D_GNU_SOURCE ./xstrndup.c -o xstrndup.o ./xstrndup.c: In function 'xstrndup': ./xstrndup.c:51:16: warning: implicit declaration of function 'strnlen' [-Wimplicit-function-declaration] size_t len = strnlen (s, n); ^ This happens because libiberty.h uses incorrect guards for the prototype of strnlen: #if defined (HAVE_DECL_STRNLEN) && !HAVE_DECL_STRNLEN extern size_t strnlen (const char *, size_t); #endif It should use HAVE_STRNLEN instead, because that's the only strnlen-related macro defined in config.g when strnlen is probed by the configure script. Thanks.
MinGW compilation warnings in libiberty's waitpid.c
When compiling libiberty (as part of GDB) with MinGW on MS-Windows, I see the following warning: gcc -c -DHAVE_CONFIG_H -O2 -gdwarf-4 -g3 -D__USE_MINGW_ACCESS -I. -I./../include -W -Wall -Wwrite-strings -Wc++-compat -Wstrict-prototypes -pedantic -D_GNU_SOURCE ./waitpid.c -o waitpid.o ./waitpid.c: In function 'waitpid': ./waitpid.c:31:18: warning: implicit declaration of function 'wait' [-Wimplicit-function-declaration] int wpid = wait(stat_loc); ^ The file waitpid.c should not be built on MinGW, as it is not needed on Windows, and will not work if the function is called (because there's no 'wait' function on MS-Windows).
MinGW compilation warnings in libiberty's include/environ.h
When compiling libiberty (as part of GDB) with MinGW on MS-Windows, I see the following warning: gcc -c -DHAVE_CONFIG_H -O2 -gdwarf-4 -g3 -D__USE_MINGW_ACCESS -I. -I./../include -W -Wall -Wwrite-strings -Wc++-compat -Wstrict-prototypes -pedantic -D_GNU_SOURCE ./setenv.c -o setenv.o In file included from ./setenv.c:64:0: ./../include/environ.h:30:1: warning: function declaration isn't a prototype [-Wstrict-prototypes] extern char **environ; ^ This was already reported 4 years ago, here: https://gcc.gnu.org/ml/gcc-patches/2013-03/msg00471.html and was solved back then. But it looks like the offending code was copied to include/environ.h without the fix, and the warning is thus back. The problem is with this code in environ.h: #ifndef HAVE_ENVIRON_DECL # ifdef __APPLE__ # include # define environ (*_NSGetEnviron ()) # else extern char **environ; # endif # define HAVE_ENVIRON_DECL #endif which causes the MinGW compiler to see the declaration of environ, whereas MinGW's stdlib.h has this: #ifdef __MSVCRT__ # define _environ (*__p__environ()) extern _CRTIMP __cdecl __MINGW_NOTHROW char ***__p__environ(void); # define _wenviron (*__p__wenviron()) extern _CRTIMP __cdecl __MINGW_NOTHROW wchar_t ***__p__wenviron(void); #else /* ! __MSVCRT__ */ # ifndef __DECLSPEC_SUPPORTED # define _environ (*_imp___environ_dll) extern char ***_imp___environ_dll; # else /* __DECLSPEC_SUPPORTED */ # define _environ _environ_dll __MINGW_IMPORT char ** _environ_dll; # endif /* __DECLSPEC_SUPPORTED */ #endif /* ! __MSVCRT__ */ #define environ _environ Can this be fixed again, please? The solution, as back then, is this: #ifndef HAVE_ENVIRON_DECL # ifdef __APPLE__ # include # define environ (*_NSGetEnviron ()) # else # ifndef environ extern char **environ; # endif # endif # define HAVE_ENVIRON_DECL #endif Thanks.
Re: Change license of filenames.h to LGPL
> From: Ozkan Sezer > Date: Thu, 29 Sep 2016 00:09:19 +0300 > Cc: Alexandre Oliva , gcc-patches@gcc.gnu.org > > On 9/28/16, Eli Zaretskii wrote: > >> From: Alexandre Oliva > >> Cc: gcc-patches@gcc.gnu.org > >> Date: Wed, 28 Sep 2016 16:03:02 -0300 > >> > >> Does that work for everyone involved? > > > > Except that no one will reimburse me for the time I wasted talking to > > several people, with eventually null result... > > > > FWIW, you have my thanks for at least helping my case. It's worth a lot to me, and you are welcome. I just hoped to actually do what you requested, not just talk about it. Now I'm in a situation where, after being authorized to make the change by whom I consider the legal custodian of that file's license, I face people who, while not being opposed to the change, don't actually want to do that. That doesn't sound right to me.
Re: Change license of filenames.h to LGPL
> From: Alexandre Oliva > Cc: gcc-patches@gcc.gnu.org > Date: Wed, 28 Sep 2016 16:03:02 -0300 > > Does that work for everyone involved? Except that no one will reimburse me for the time I wasted talking to several people, with eventually null result...
Re: Change license of filenames.h to LGPL
> From: Florian Weimer > Cc: DJ Delorie , gcc-patches@gcc.gnu.org, seze...@gmail.com > Date: Wed, 28 Sep 2016 16:43:53 +0200 > > * Eli Zaretskii: > > > If my arithmetics is correct, about 70% of its files is LGPL, the > > rest GPL. Which doesn't keep many GNU projects under GPL from using > > Gnulib. > > Sorry, I don't understand. Surely anything released under the LGPL by > the FSF can be upgraded to the current GPLv3? First upgrade to the > latest LGPL, then switch over to the GPLv3? > > (I assume that the FSF releases their works under the “any later > version” regime.) The above was in response to DJ's questions up-thread: > > Because Ozkan wants to use it in an otherwise LGPL package. > > But then the implementation would need relicensing as well, wouldn't > it? > > Having both under different licenses is just confusing. Did I misunderstand the question?
Re: Change license of filenames.h to LGPL
> From: DJ Delorie > Cc: gcc-patches@gcc.gnu.org, e...@gnu.org, f...@deneb.enyo.de > Date: Tue, 27 Sep 2016 15:45:28 -0400 > > > I wonder if us relicensing our modified copy would apply to your old > copy. I mean, are we sure RMS knows you're also relicensing an old > copy, and that the current copy is being relicensed only to avoid future > issues. If we're only doing it to document the decision, the fact that > hashtab.h and filename_cmp.c are still GPL mostly negates the > effectiveness of our change anyway. > > (i.e. it seems like you can get what you need whether we relicense ours > or not, and relicensing ours doesn't have much actual effect). I see no reason why setting the record straight about license compatibility should be an issue for us. Better late than never, right?
Re: Change license of filenames.h to LGPL
> From: DJ Delorie > Cc: f...@deneb.enyo.de, gcc-patches@gcc.gnu.org, seze...@gmail.com > Date: Tue, 27 Sep 2016 15:23:46 -0400 > > > Why would it need to > > change? It's perfectly okay to link GPL code with LGPL code, we do > > this all the time with libgcc, no? Or am I missing something? > > libgcc has an exception that covers most of those cases; be careful when > comparing those to your (his) use case. OK, then take Gnulib as a better example. If my arithmetics is correct, about 70% of its files is LGPL, the rest GPL. Which doesn't keep many GNU projects under GPL from using Gnulib.
Re: Change license of filenames.h to LGPL
> From: DJ Delorie > Cc: f...@deneb.enyo.de, gcc-patches@gcc.gnu.org, seze...@gmail.com > Date: Tue, 27 Sep 2016 15:23:46 -0400 > > Eli Zaretskii writes: > >> But then the implementation would need relicensing as well, wouldn't > >> it? > > > > Which implementation? of Ozkan's library? > > libiberty's filename_cmp.c is GPL and implements two of the functions in > filenames.h; if those are why he's using it, then it's still GPL unless > filename_cmp.c is changed also. I'm guessing he only wants the macros and will delete the rest. (The original file as written by me years ago had nothing but those few macros.) But I will leave it to Ozkan to give the definitive answer.
Re: Change license of filenames.h to LGPL
> From: DJ Delorie > Cc: gcc-patches@gcc.gnu.org, seze...@gmail.com > Date: Tue, 27 Sep 2016 15:00:46 -0400 > > > Eli Zaretskii writes: > > Because Ozkan wants to use it in an otherwise LGPL package. > > Ok, but that doesn't say why it's different. That reason could apply to > any header in there. Do we need to convert all headers there to LGPL? I understand that Ozkan only needs that one. Ozkan? > Is this "otherwise LGPL package" in one of our repos, or elsewhere? Is > he aware that filenames.h includes hashtab.h, which is GPL? > > (I'm not opposed to the change, just trying to make sure I understand > it's scope) I'll let Ozkan answer these questions. But if someone wants to know which libraries need that, one of them is this: http://libtimidity.sourceforge.net/
Re: Change license of filenames.h to LGPL
> From: Florian Weimer > Cc: DJ Delorie , gcc-patches@gcc.gnu.org, seze...@gmail.com > Date: Tue, 27 Sep 2016 20:54:59 +0200 > > >> Most of the files in include/ are GPL, not LGPL. Why is this one > >> different? > > > > Because Ozkan wants to use it in an otherwise LGPL package. > > But then the implementation would need relicensing as well, wouldn't > it? Which implementation? of Ozkan's library? Why would it need to change? It's perfectly okay to link GPL code with LGPL code, we do this all the time with libgcc, no? Or am I missing something? > Having both under different licenses is just confusing. AFAIK, that ship sailed a long time ago, in several projects, including Binutils and GDB.
Re: Change license of filenames.h to LGPL
> Cc: gcc-patches@gcc.gnu.org, seze...@gmail.com > From: Jeff Law > Date: Tue, 27 Sep 2016 12:36:11 -0600 > > On 09/27/2016 11:52 AM, DJ Delorie wrote: > > > > Most of the files in include/ are GPL, not LGPL. Why is this one > > different? > Right. ANd it's not like this file inserts anything of significance > into the resulting object code. I'd really like to see more rationale > behind the request for a license change. See my other message. In the original request I reported that this change was already approved by Richard Stallman, who asked these same questions.
Re: Change license of filenames.h to LGPL
> From: DJ Delorie > Cc: gcc-patches@gcc.gnu.org, seze...@gmail.com > Date: Tue, 27 Sep 2016 13:52:10 -0400 > > > Most of the files in include/ are GPL, not LGPL. Why is this one > different? Because Ozkan wants to use it in an otherwise LGPL package.
Change license of filenames.h to LGPL
Hi, I was asked by Ozkan Sezer (CC'ed) whether I'd agree to relicense include/filenames.h under LGPL2+ instead of GPL2+. I talked to Richard Stallman (in private email), and he authorized the change. So now I'm proposing the corresponding change to you. Thanks. P.S. Please CC me on any responses, as I'm not subscribed to this list. == Relicense include/filenames.h as LGPL2+. include/ChangeLog: 2016-09-24 Eli Zaretskii * filenames.h: Relicense as LGPL 2.1 or later. diff --git a/include/filenames.h b/include/filenames.h index 44553e4..b933bcf 100644 --- a/include/filenames.h +++ b/include/filenames.h @@ -10,18 +10,19 @@ This file is part of BFD, the Binary File Descriptor library. This program is free software; you can redistribute it and/or modify -it under the terms of the GNU General Public License as published by -the Free Software Foundation; either version 2 of the License, or -(at your option) any later version. +it under the terms of the GNU Lesser General Public License as +published by the Free Software Foundation; either version 2.1 of the +License, or (at your option) any later version. This program is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -GNU General Public License for more details. +GNU Lesser General Public License for more details. -You should have received a copy of the GNU General Public License -along with this program; if not, write to the Free Software -Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston, MA 02110-1301, USA. */ +You should have received a copy of the GNU Lesser General Public +License along with this program; if not, write to the Free Software +Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston, MA +02110-1301, USA. */ #ifndef FILENAMES_H #define FILENAMES_H
Fix "make TAGS" in libdecnumber
"make TAGS" in the GDB source tree fails because libdecnumber doesn't have a TAGS target. This patch fixes that: commit 53bef1c10759f1fd7faf675459871b2f4cc12e53 Author: Eli Zaretskii Date: Thu Jan 22 21:07:31 2015 +0200 Another part of fixing "make TAGS". libdecnumber/ 2015-01-22 Eli Zaretskii * Makefile.in (TAGS): New target, to avoid failures in "make TAGS". diff --git a/libdecnumber/ChangeLog b/libdecnumber/ChangeLog index 65d20fc..ec6ea1a 100644 --- a/libdecnumber/ChangeLog +++ b/libdecnumber/ChangeLog @@ -1,3 +1,7 @@ +2015-01-22 Eli Zaretskii + + * Makefile.in (TAGS): New target, to avoid failures in "make TAGS". + 2014-01-23 Marek Polacek PR c/59871 diff --git a/libdecnumber/Makefile.in b/libdecnumber/Makefile.in index b6f3842..8dbada9 100644 --- a/libdecnumber/Makefile.in +++ b/libdecnumber/Makefile.in @@ -167,6 +167,8 @@ maintainer-clean: distclean @echo "it deletes files that may require special tools to rebuild." -rm -f $(srcdir)/configure $(srcdir)/aclocal.m4 +TAGS: + check: installcheck: dvi:
Re: Fix a MinGW warning in libiberty/strerror.c
> Date: Fri, 16 Jan 2015 16:39:20 -0500 > From: DJ Delorie > CC: ktiet...@googlemail.com, gcc-patches@gcc.gnu.org, > gdb-patc...@sourceware.org > > > > Thanks. Do I need to hear from someone else approving this, or can I > > go ahead and commit? > > Go ahead and commit. Thanks. Last-minute thought, in case I'm confused: you mean commit to the sourceware Git repo, right? If not, where?
Re: Fix a MinGW warning in libiberty/strerror.c
> Date: Fri, 16 Jan 2015 12:34:25 +0100 > From: Kai Tietz > Cc: GCC Patches , > "gdb-patc...@sourceware.org" > > Hi Eli, > > patch is reasonable and ok for me. Thanks. Do I need to hear from someone else approving this, or can I go ahead and commit?
Re: Fix a MinGW warning in libiberty/strerror.c
Ping! > Date: Fri, 02 Jan 2015 12:54:47 +0200 > From: Eli Zaretskii > > When compiling GDB 7.8.1, I get this warning in libiberty: > > gcc -c -DHAVE_CONFIG_H -O0 -g3 -D__USE_MINGW_ACCESS -I. -I./../include > -W -Wall -Wwrite-strings -Wc++-compat -Wstrict-prototypes -pedantic > ./strerror.c -o strerror.o > ./strerror.c:472:12: warning: '_sys_nerr' redeclared without dllimport > attribute: previous dllimport ignored [-Wattributes] > ./strerror.c:473:14: warning: '_sys_errlist' redeclared without > dllimport attribute: previous dllimport ignored [-Wattributes] > > This happens because the MinGW system headers have some special magic > for these variables, which are imported from a system shared library. > > The solution I propose is to refrain from declaring variables that are > actually macros, because this should be a sign that something tricky > is going on: > > --- libiberty/strerror.c~02014-06-11 18:34:41 +0300 > +++ libiberty/strerror.c 2014-12-30 08:12:00 +0200 > @@ -469,8 +469,13 @@ > > #else > > + > +#ifndef sys_nerr > extern int sys_nerr; > +#endif > +#ifndef sys_errlist > extern char *sys_errlist[]; > +#endif > > #endif > > > OK to commit this (with a suitable ChangeLog entry)? >
Fix a MinGW warning in libiberty/strerror.c
When compiling GDB 7.8.1, I get this warning in libiberty: gcc -c -DHAVE_CONFIG_H -O0 -g3 -D__USE_MINGW_ACCESS -I. -I./../include -W -Wall -Wwrite-strings -Wc++-compat -Wstrict-prototypes -pedantic ./strerror.c -o strerror.o ./strerror.c:472:12: warning: '_sys_nerr' redeclared without dllimport attribute: previous dllimport ignored [-Wattributes] ./strerror.c:473:14: warning: '_sys_errlist' redeclared without dllimport attribute: previous dllimport ignored [-Wattributes] This happens because the MinGW system headers have some special magic for these variables, which are imported from a system shared library. The solution I propose is to refrain from declaring variables that are actually macros, because this should be a sign that something tricky is going on: --- libiberty/strerror.c~0 2014-06-11 18:34:41 +0300 +++ libiberty/strerror.c2014-12-30 08:12:00 +0200 @@ -469,8 +469,13 @@ #else + +#ifndef sys_nerr extern int sys_nerr; +#endif +#ifndef sys_errlist extern char *sys_errlist[]; +#endif #endif OK to commit this (with a suitable ChangeLog entry)?
Re: [PATCH 1/2] Windows libibery: Don't quote args unnecessarily
> Date: Sat, 19 Apr 2014 16:23:33 -0400 (EDT)
> From: Kai Tietz
> Cc: gcc-patches@gcc.gnu.org, ktiet...@gmail.com,
> "binut...@sourceware.org Development" ,
> gdb-patc...@sourceware.org
>
> > + /* We only quote arguments that contain spaces, \n \t \v or "
> > characters
> > +to prevent wasting 2 chars per argument of the CreateProcess 32k char
> > +limit We need only escape embedded double-quotes and immediately
> > preceeding backslash characters. A sequence of backslach characters
> > that is not follwed by a double quote character will not be
> > escaped. */
> > + needs_quotes = 0;
> >for (j = 0; argv[i][j]; j++)
> > {
> > + if (argv[i][j] == ' ' || argv[i][j] == '\n' ||
> > + argv[i][j] == '\t' || argv[i][j] == '"' )
> > + {
> Here seems to be an intend issue.
> > + needs_quotes = 1;
> > + }
I think you can omit the \n case, since command arguments on Windows
cannot possibly include newlines. Also, the comment speaks about \v,
but I see no code to handle that (and am not sure you should bother in
that case as well).
> Patch itself makes sense.
Yes, I agree.
Re: Fix a MinGW warning in libiberty/setenv.c
> Date: Wed, 13 Mar 2013 11:52:48 -0700 > From: Ian Lance Taylor > Cc: gdb-patc...@sourceware.org, d...@redhat.com, gcc-patches@gcc.gnu.org > > On 3/13/13, Eli Zaretskii wrote: > > > > #ifdef __MSVCRT__ > >extern _CRTIMP char *** __cdecl __MINGW_NOTHROW __p__environ(void); > >extern _CRTIMP wchar_t *** __cdecl __MINGW_NOTHROW > > __p__wenviron(void); > > # define _environ (*__p__environ()) > > # define _wenviron (*__p__wenviron()) > > #else /* ! __MSVCRT__ */ > > #endif /* ! __MSVCRT__ */ > > > > #define environ _environ > > Cool. > > >and setenv.c does this: > > > > #ifndef HAVE_ENVIRON_DECL > > extern char **environ; > > #endif > > > >Solution: Add a guard: > > This is OK with a ChangeLog entry. Thanks, committed thusly: 2013-03-17 Eli Zaretskii * setenv.c [!HAVE_ENVIRON_DECL]: Avoid declaring environ if it is a macro, as this causes compiler warnings with MinGW. Index: libiberty/setenv.c === RCS file: /cvs/src/src/libiberty/setenv.c,v retrieving revision 1.10 diff -u -r1.10 setenv.c --- libiberty/setenv.c 3 Feb 2011 07:23:59 - 1.10 +++ libiberty/setenv.c 17 Mar 2013 19:03:07 - @@ -63,8 +63,11 @@ #define __environ environ #ifndef HAVE_ENVIRON_DECL +/* MinGW defines environ to call a function. */ +#ifndef environ extern char **environ; #endif +#endif #undef setenv #undef unsetenv
Fox a MinGW warning in libiberty/setenv.c
I get this compiling the latest pretest of GDB 7.6 with MinGW: gcc -c -DHAVE_CONFIG_H -O2 -gdwarf-2 -g3 -D__USE_MINGW_ACCESS -I. -I./../include -W -Wall -Wwrite-strings -Wc++-compat -Wstrict-prototypes -pedantic ./setenv.c -o setenv.o ./setenv.c:66:1: warning: function declaration isn't a prototype [-Wstrict-prototypes] This happens because MinGW's stdlib.h has this: #ifdef __MSVCRT__ extern _CRTIMP char *** __cdecl __MINGW_NOTHROW __p__environ(void); extern _CRTIMP wchar_t *** __cdecl __MINGW_NOTHROW __p__wenviron(void); # define _environ (*__p__environ()) # define _wenviron (*__p__wenviron()) #else /* ! __MSVCRT__ */ #endif /* ! __MSVCRT__ */ #define environ _environ and setenv.c does this: #ifndef HAVE_ENVIRON_DECL extern char **environ; #endif Solution: Add a guard: --- libiberty/setenv.c~ 2011-02-03 09:23:59.0 +0200 +++ libiberty/setenv.c 2013-03-13 13:22:49.085187200 +0200 @@ -63,8 +63,10 @@ #define __environ environ #ifndef HAVE_ENVIRON_DECL +#ifndef environ extern char **environ; #endif +#endif OK to commit (with a suitable ChangeLog entry)?
Re: [RFA libiberty, gdb] Add hashtab support to filename_ncmp.c and use it in gdb.
> Date: Fri, 13 Jul 2012 14:26:15 -0700 > From: Doug Evans > Cc: d...@redhat.com, gcc-patches@gcc.gnu.org, gdb-patc...@sourceware.org > > > Sorry if I'm missing something, but why do we need to advertise such a > > function at all? Given that libiberty already provides filename_cmp, > > isn't it trivial to write something like filename_eq whenever someone > > needs to use hashes of file names? > > It's a "matched set" with filename_hash. > The hashtab.c constructors take a hash_f function pointer and an eq_f > function pointer. I understand all that, but why would the eq_f function need to be an external function on its own? E.g., if we were to write a qsort replacement, would we have a suitable string comparison function declared extern, when it is a trivial wrapper around strcmp?
Re: [RFA libiberty, gdb] Add hashtab support to filename_ncmp.c and use it in gdb.
> Date: Fri, 13 Jul 2012 12:36:44 -0700 > From: Doug Evans > Cc: gcc-patches@gcc.gnu.org, gdb-patc...@sourceware.org > > On Fri, Jul 13, 2012 at 12:21 PM, DJ Delorie wrote: > > > > I think it's confusing to have filename_cmp and filename_eq that do > > basically the same thing. Perhaps filename_eq should be > > filename_cmp_v or filename_cmp_hash or something, to indicate that > > it's *supposed* to be the same functionality as filename_cmp but with > > a different signature? > > To be clear, filename_cmp is to strcmp as filename_eq is to streq. > > ref: STREQ in libiberty/regex.c: > # define STREQ(s1, s2) ((strcmp (s1, s2) == 0)) > > Given that, I think the names are fine as is, but I'm happy to change them. Sorry if I'm missing something, but why do we need to advertise such a function at all? Given that libiberty already provides filename_cmp, isn't it trivial to write something like filename_eq whenever someone needs to use hashes of file names?
Re: [RFA/libiberty] Darwin has case-insensitive filesystems
> Date: Wed, 15 Jun 2011 06:59:11 -0400 > From: Robert Dewar > CC: vinsc...@redhat.com, d...@redhat.com, pins...@gmail.com, > brobec...@adacore.com, gcc-patches@gcc.gnu.org, gdb-patc...@sourceware.org > > > or some piece of code in the toolchain arbitrarily changed the > > case of a filename. I don't mind punishing people for that. They > > have to learn that on a proper operating system file names are > > case-sensitive! > > This kind of unix arrogance leads to junk unusable software on > windows. It's really important not to visit your unix predjudices > on windows users. After all we feel the same way in return, I > find Unix systems complete junk for many reasons, one of which > is the very annoying case sensitive viewpoint, but I do not > translate my feelings into silly suggestions for making > software malfunction on Unix. You should not make this mistake > in a reverse direction. I cannot agree more.
Re: [RFA/libiberty] Darwin has case-insensitive filesystems
> Date: Wed, 15 Jun 2011 10:22:36 +0200 > From: Corinna Vinschen > Cc: Andrew Pinski , brobec...@adacore.com, > gcc-patches@gcc.gnu.org, gdb-patc...@sourceware.org > > Talking about case-insensitive comparison, the filename_cmp and > filename_ncmp functions don't work for multibyte codesets, only for > singlebyte codesets. Given that UTF-8 is standard nowadays, shouldn't > these functions be replaced with multibyte-aware versions? I agree, but if we go that way, shouldn't we support UTF-16, which is used by the native Windows APIs? Windows does not use UTF-8 for file names.
Re: RFC: Issue about assumption of DOSish file-system for cygwin
> Date: Tue, 22 Mar 2011 19:31:58 +0100 > From: Kai Tietz > Cc: gdb-patc...@sourceware.org, Dave Korn , > GCC Patches , Binutils > > > IS_ABSOLUTE_PATH already allows that when HAVE_DOS_BASED_FILE_SYSTEM is > > true. > > Hmm, interesting. I look in more detail. As for DOSish file-system a > leading slash/backslash isn't necessarily an absolute path. Yes, it is, for all practical purposes in the context of this discussion.
Re: [patch libiberty include]: Add additional helper functions for directory-separator searching
> From: Pedro Alves > Date: Wed, 9 Mar 2011 12:58:38 + > Cc: gdb-patc...@sourceware.org, > d...@redhat.com, > ktiet...@googlemail.com, > binut...@sourceware.org, > gcc-patches@gcc.gnu.org > > > > The one's left are: 1 in a linux-native only file (never cares > > > for other filesystem semantics), and a couple in the coff and > > > mdebug readers. The latter could be rewritten in terms of > > > lbasename, but I'm not sure whether gcc outputs a literal '/' in > > > that case even when building on mingw. If so, and we changed them, > > > we'd be breaking reading these files on Windows > > > > Sorry, I don't understand how would that break on Windows. Could you > > elaborate? And what "couple of coff and mdebug readers" did you have > > in mind? > > Sorry, in the hurry, I had a (another) brain cramp. Wouldn't break. > Still it'd be useless to change this _if_ gcc hardcodes '/'. Dunno > whether it does. At least on MinGW, GCC simply uses whatever was passed on the command line. I tested that by compiling the same source file, passing it to GCC with different flavors of slashes, including mixed ones. Then in GDB I typed "info sources" and saw the source file with exactly the same flavor of slashes as what I typed on the GCC command line. Funnily enough, when the file name given to GCC includes at least one backslash, "info sources" shows the same file twice, like this: (gdb) info sources Source files for which symbols have been read in: Source files for which symbols will be read in on demand: d:/usr/eli/data/dbw.c, d:\usr\eli/data\dbw.c This is with GDB 7.2 and GCC 3.4.2. That means we compare files with strcmp/strcasecmp somewhere, and don't know that / and \ are equivalent here. Or maybe it's a bug in the ancient version of GCC I use.
Re: [patch libiberty include]: Add additional helper functions for directory-separator searching
> From: Pedro Alves > Date: Wed, 9 Mar 2011 11:46:36 + > Cc: gdb-patc...@sourceware.org, > d...@redhat.com, > ktiet...@googlemail.com, > binut...@sourceware.org, > gcc-patches@gcc.gnu.org > > On Wednesday 09 March 2011 05:29:09, Eli Zaretskii wrote: > > > Actually, is there any case where lbasename wouldn't > > > work instead of filename_dirrchr? > > > > Almost: lbasename returns a pointer one character after the last > > slash. It also skips the drive letter on DOS/Windows (which might be > > TRT, actually). > > I meant a valid use case in the code bases. Sorry for my misunderstanding. > Might as well cook up a (gdb) patch. Find it pasted below. Does it > look good to you? Yes, looks fine. Thanks. > The one's left are: 1 in a linux-native only file (never cares > for other filesystem semantics), and a couple in the coff and > mdebug readers. The latter could be rewritten in terms of > lbasename, but I'm not sure whether gcc outputs a literal '/' in > that case even when building on mingw. If so, and we changed them, > we'd be breaking reading these files on Windows Sorry, I don't understand how would that break on Windows. Could you elaborate? And what "couple of coff and mdebug readers" did you have in mind?
Re: [patch libiberty include]: Add additional helper functions for directory-separator searching
> From: Pedro Alves > Date: Tue, 8 Mar 2011 22:37:59 + > Cc: DJ Delorie , > Eli Zaretskii , > ktiet...@googlemail.com, > binut...@sourceware.org, > gcc-patches@gcc.gnu.org > > Actually, is there any case where lbasename wouldn't > work instead of filename_dirrchr? Almost: lbasename returns a pointer one character _after_ the last slash. It also skips the drive letter on DOS/Windows (which might be TRT, actually). It would be reasonable to rewrite filename_dirrchr in terms of lbasename, though, by backing up the pointer returned by lbasename if it points to a slash, and otherwise returning NULL. The case of "d:foo" should also be resolved (probably, return a pointer to the colon).
Re: [patch libiberty include]: Add additional helper functions for directory-separator searching
> Date: Tue, 8 Mar 2011 14:41:00 -0500 > From: DJ Delorie > CC: ktiet...@googlemail.com, binut...@sourceware.org, > gdb-patc...@sourceware.org, gcc-patches@gcc.gnu.org > > > I see no harm in having both, for completeness, though. I don't see any need for completeness in this case. But it's your call. I still think that the documentation should be fixed, though.
Re: [patch libiberty include]: Add additional helper functions for directory-separator searching
> Date: Tue, 8 Mar 2011 19:51:14 +0100 > From: Kai Tietz > Cc: Pedro Alves , gdb-patc...@sourceware.org, > gcc-patches@gcc.gnu.org, binut...@sourceware.org > > > In my experience, the strchr analog is not needed, only the strrchr > > one (which could be used quite a lot). The few places that use strchr > > now should actually be rewritten to search from the end, because > > that's what they need. > > > > Here I am not that sure. For example in gcc's gengtype.c > (read_input_list) is a use-case for strchr on filenames, which can't > be expressed by strrchr. I don't see any reason to have in libiberty a function that has a single use.
Re: [patch libiberty include]: Add additional helper functions for directory-separator searching
> From: Pedro Alves > Date: Tue, 8 Mar 2011 13:33:02 + > Cc: Kai Tietz , > gcc-patches@gcc.gnu.org, > Eli Zaretskii , > binut...@sourceware.org > > On Tuesday 08 March 2011 12:48:11, Kai Tietz wrote: > > > Well, a better example is elfstab_offset_sections() in elfread.c. > > /* The ELF symbol info doesn't include path names, so strip the path > (if any) from the psymtab filename. */ > while (0 != (p = strchr (filename, '/'))) > filename = p + 1; > > Looks like its looking for the last path separator, so > it might as well use filename_dirrchr instead. Exactly. > > Another is in find_file_and_directory() in dwarf2read.c file. > > Workaround for Irix. Certainly that '/' should not depend > on the host gdb is running on. It actually should use IS_ABSOLUTE_FILE_NAME, if any portability enhancement is needed here. In my experience, the strchr analog is not needed, only the strrchr one (which could be used quite a lot). The few places that use strchr now should actually be rewritten to search from the end, because that's what they need.
Re: [patch libiberty include]: Add additional helper functions for directory-separator searching
> Date: Tue, 8 Mar 2011 16:29:51 +0100 > From: Kai Tietz > > Umm, sorry. I found a wrong copy & paste. So I re-sent the corrected > patch. Additionally I adjuste the changes in Makefile.in so, that > alphabetic order remains. I think we don't need filename_dirchr, only filename_dirrchr. And you didn't change anything in the documentation to address my comments earlier today.
Re: [patch libiberty include]: Add additional helper functions for directory-separator searching
> Date: Tue, 8 Mar 2011 12:25:37 +0100 > From: Kai Tietz > Cc: binut...@sourceware.org, gdb-patc...@sourceware.org, > gcc-patches@gcc.gnu.org > > > Btw, why do we need filename_dirchr? The use case for > > filename_dirrchr is clear, but in what situations will we need the > > other one? > > As the comment notes. strchr/strrchr searches for one character. This > is for unix-file-system normally slash. On DOS based file-systems > there are two characters representing a directory-separator. Slash and > Backslash. Therefore this routine takes care that both are handled > similiar to a single character searching. We are miscommunicating. I was asking when would a program want to find the _first_ directory separator character in a file name. Searching for the last one is a frequent use case: when you want to create a file in the same directory as another, or when you are looking for a basename of a file. But when do you need the first slash? > >> + if (!r || (r2 && r2 < r)) > > > > Why do you test for r2 being non-NULL? You are not going to > > dereference it in the next comparison, and NULL is comparable as any > > other value. > > As if we found slash, we don't want to override function's result by > backslash not found. If the null-check wouldn't be present condition > would be always true for r2 == NULL as, NULL is always less then a > pointer. But r shall be modified only if r2 (backslash) was found > before r (slash). > (same logic but here from right to left for the strrchr-case) But in strrchr-case, r2 cannot be greater than r1 if it is NULL, right?
Re: [patch libiberty include]: Add additional helper functions for directory-separator searching
> Date: Tue, 8 Mar 2011 11:56:45 +0100
> From: Kai Tietz
>
> +@deftypefn Extension int filename_dirchr (const char *@var{p})
> +
> +The returned value is similar to what @code{strchr} would return for
> +searching for a directory separator.
> +
> +This function does not normalize file name. However, it does handle
> +the fact that on DOS-like file systems, forward and backward slashes
> +are directory separators.
This is very mysterious. The documentation should explain how this is
"handled", or else the user will have no choice but to look in the
sources. And description "by similarity" doesn't help, because this
function is obviously different from strchr in _some_ ways, but you
don't say how.
While at that, explain the problem this solves, or else the raison
d'etre of this function will not be understood. We do want this
function to be used instead of just strchr, don't we? For it to be
used, its purpose and advantages should be well understood.
Btw, why do we need filename_dirchr? The use case for
filename_dirrchr is clear, but in what situations will we need the
other one?
> + if (!r || (r2 && r2 < r))
Why do you test for r2 being non-NULL? You are not going to
dereference it in the next comparison, and NULL is comparable as any
other value.
> +@deftypefn Extension int filename_dirrchr (const char *@var{p})
> +
> +The returned value is similar to what @code{strrchr} would return for
> +searching for a directory separator.
> +
> +This function does not normalize file name. However, it does handle
> +the fact that on DOS-like file systems, forward and backward slashes
> +are directory separators.
Same comments about this doc.
> + if (!r || (r2 && r2 > r))
And same comment here about testing r2 for non-NULL value.
Please also wait for others to review, as I'm not authorized to
approve the changes.
Thanks for working on this.
