[PATCH] hurd: Add shared mig declarations

[PATCH] hurd: Add shared mig declarations

[Samuel Thibault]

Fixes

exc_request_S.c:177:24: error: no previous declaration for ‘exc_server’ [-Werror=missing-declarations]
  177 | mig_external boolean_t exc_server

gdb/ChangeLog:

	* config/i386/i386gnu.mn [%_S.o %_U.o] (COMPILE.post): Add
	"-include gnu-nat-mig.h".
	* gnu-nat-mig.h: New file.
	* gnu-nat.c: Include "gnu-nat-mig.h".
	(exc_server, msg_reply_server, notify_server,
	process_reply_server): Remove declarations.

Index: binutils-gdb/gdb/config/i386/i386gnu.mn
===================================================================
--- binutils-gdb.orig/gdb/config/i386/i386gnu.mn
+++ binutils-gdb/gdb/config/i386/i386gnu.mn
@@ -21,7 +21,7 @@ MIGCOM = $(MIG) -cc cat - /dev/null
 	| $(MIGCOM) -sheader /dev/null -server /dev/null -user $*_U.c -header $*_U.h
 
 # MIG stubs are not yet ready for C++ compilation.
-%_S.o %_U.o : COMPILE.post += -x c
+%_S.o %_U.o : COMPILE.post += -x c -include gnu-nat-mig.h
 
 NAT_GENERATED_FILES = notify_S.h notify_S.c \
 	process_reply_S.h process_reply_S.c \
Index: binutils-gdb/gdb/gnu-nat-mig.h
===================================================================
--- /dev/null
+++ binutils-gdb/gdb/gnu-nat-mig.h
@@ -0,0 +1,33 @@
+/* Common things used by the various *gnu-nat.c files
+   Copyright (C) 2020 Free Software Foundation, Inc.
+
+   Written by Samuel Thibault <samuel.thibault@ens-lyon.org>
+
+   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 3 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.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#ifndef GNU_NAT_MIG_H
+#define GNU_NAT_MIG_H
+
+#include <mach/boolean.h>
+#include <mach/message.h>
+
+boolean_t exc_server (mach_msg_header_t *InHeadP, mach_msg_header_t *OutHeadP);
+boolean_t msg_reply_server (mach_msg_header_t *InHeadP,
+			    mach_msg_header_t *OutHeadP)
+boolean_t notify_server (mach_msg_header_t *InHeadP,
+			 mach_msg_header_t *OutHeadP)
+boolean_t process_reply_server (mach_msg_header_t *InHeadP,
+				mach_msg_header_t *OutHeadP)
+
+#endif /* GNU_NAT_MIG_H */
Index: binutils-gdb/gdb/gnu-nat.c
===================================================================
--- binutils-gdb.orig/gdb/gnu-nat.c
+++ binutils-gdb/gdb/gnu-nat.c
@@ -46,6 +46,9 @@ extern "C"
 #include <hurd/sigpreempt.h>
 
 #include <portinfo.h>
+
+/* MIG stubs are not yet ready for C++ compilation.  */
+#include "gnu-nat-mig.h"
 }
 
 #include "defs.h"
@@ -1430,12 +1433,6 @@ struct inf *gnu_current_inf = 0;
    multi-threaded, we don't bother to lock this.  */
 static struct inf *waiting_inf;
 
-/* MIG stubs are not yet ready for C++ compilation.  */
-extern "C" int exc_server (mach_msg_header_t *, mach_msg_header_t *);
-extern "C" int msg_reply_server (mach_msg_header_t *, mach_msg_header_t *);
-extern "C" int notify_server (mach_msg_header_t *, mach_msg_header_t *);
-extern "C" int process_reply_server (mach_msg_header_t *, mach_msg_header_t *);
-
 /* Wait for something to happen in the inferior, returning what in STATUS.  */
 
 ptid_t

Re: [PATCH] hurd: Add shared mig declarations

[Samuel Thibault]

Samuel Thibault, le sam. 30 mai 2020 20:23:18 +0200, a ecrit:
> +boolean_t exc_server (mach_msg_header_t *InHeadP, mach_msg_header_t *OutHeadP);
> +boolean_t msg_reply_server (mach_msg_header_t *InHeadP,
> +			    mach_msg_header_t *OutHeadP)
> +boolean_t notify_server (mach_msg_header_t *InHeadP,
> +			 mach_msg_header_t *OutHeadP)
> +boolean_t process_reply_server (mach_msg_header_t *InHeadP,
> +				mach_msg_header_t *OutHeadP)

Sorry, it was not refreshed. There are ';' at the end of these three
lines of course.

Re: [PATCH] hurd: Add shared mig declarations

[Pedro Alves]

On 5/30/20 7:23 PM, Samuel Thibault wrote:
> +++ binutils-gdb/gdb/gnu-nat-mig.h
> @@ -0,0 +1,33 @@
> +/* Common things used by the various *gnu-nat.c files
> +   Copyright (C) 2020 Free Software Foundation, Inc.
> +
> +   Written by Samuel Thibault <samuel.thibault@ens-lyon.org>

Please don't take this the wrong way, but a while ago the
GDB project (following Glibc's lead) decided that we would
not add more "contributed by", "written by", etc. notes.

See here <https://sourceware.org/gdb/wiki/ContributionChecklist#Attribution> 
and follow the link there for rationale.

Thanks,
Pedro Alves

Re: [PATCH] hurd: Add shared mig declarations

[Samuel Thibault]

Pedro Alves, le sam. 30 mai 2020 20:51:17 +0100, a ecrit:
> On 5/30/20 7:23 PM, Samuel Thibault wrote:
> > +++ binutils-gdb/gdb/gnu-nat-mig.h
> > @@ -0,0 +1,33 @@
> > +/* Common things used by the various *gnu-nat.c files
> > +   Copyright (C) 2020 Free Software Foundation, Inc.
> > +
> > +   Written by Samuel Thibault <samuel.thibault@ens-lyon.org>
> 
> Please don't take this the wrong way, but a while ago the
> GDB project (following Glibc's lead) decided that we would
> not add more "contributed by", "written by", etc. notes.
> 
> See here <https://sourceware.org/gdb/wiki/ContributionChecklist#Attribution> 
> and follow the link there for rationale.

Ah, I had read that part, and thought it was talking about the commit
log, I didn't notice it was talking about source files.

Sure I'll just drop it, I was just following the content of the existing
gnu-nat.c file.

Samuel

Re: [PATCH] hurd: Add shared mig declarations

[Simon Marchi]

On 2020-05-30 2:23 p.m., Samuel Thibault wrote:
> Fixes
> 
> exc_request_S.c:177:24: error: no previous declaration for ‘exc_server’ [-Werror=missing-declarations]
>   177 | mig_external boolean_t exc_server
> 
> gdb/ChangeLog:
> 
> 	* config/i386/i386gnu.mn [%_S.o %_U.o] (COMPILE.post): Add
> 	"-include gnu-nat-mig.h".
> 	* gnu-nat-mig.h: New file.
> 	* gnu-nat.c: Include "gnu-nat-mig.h".
> 	(exc_server, msg_reply_server, notify_server,
> 	process_reply_server): Remove declarations.

It took me a while to understand the underlying problem.  My understanding is that
gnu-nat.c calls this function exc_server, that is defined in the generated file.  The
generated file does not provide a header with declarations, so gnu-nat.c had its own
local declaration.  Since we now use the -Wmissing-declarations warning flag, and the
definition in the generated exc_request_S.c didn't see a corresponding declaration,
it caused that build failure.  Is that correct?  If so, please add that explanation
or equivalent to the commit log.

My question now is: that MIG tool appears to generate both a header (%_S.h) and source
file (%_S.c) from defs files.  What is this header file used for, if it doesn't contain
the declaration for the functions in the source file?

Simon

Re: [PATCH] hurd: Add shared mig declarations

[Samuel Thibault]

Simon Marchi, le sam. 30 mai 2020 21:51:35 -0400, a ecrit:
> On 2020-05-30 2:23 p.m., Samuel Thibault wrote:
> > Fixes
> > 
> > exc_request_S.c:177:24: error: no previous declaration for ‘exc_server’ [-Werror=missing-declarations]
> >   177 | mig_external boolean_t exc_server
> > 
> > gdb/ChangeLog:
> > 
> > 	* config/i386/i386gnu.mn [%_S.o %_U.o] (COMPILE.post): Add
> > 	"-include gnu-nat-mig.h".
> > 	* gnu-nat-mig.h: New file.
> > 	* gnu-nat.c: Include "gnu-nat-mig.h".
> > 	(exc_server, msg_reply_server, notify_server,
> > 	process_reply_server): Remove declarations.
> 
> It took me a while to understand the underlying problem.  My understanding is that
> gnu-nat.c calls this function exc_server, that is defined in the generated file.  The
> generated file does not provide a header with declarations, so gnu-nat.c had its own
> local declaration.  Since we now use the -Wmissing-declarations warning flag, and the
> definition in the generated exc_request_S.c didn't see a corresponding declaration,
> it caused that build failure.  Is that correct?  If so, please add that explanation
> or equivalent to the commit log.

I have now added

“
We are using -Werror=missing-declarations, and the _S.h files generated
by mig do not currently include a declaration for the server routine.
gnu-nat.c used to have its own external declarations, but better just
share them between gnu-nat.c and the _S.c files.
”

> My question now is: that MIG tool appears to generate both a header (%_S.h) and source
> file (%_S.c) from defs files.  What is this header file used for, if it doesn't contain
> the declaration for the functions in the source file?

Mig does include declarations for the functions of the .c files, but
not for the server routine, I don't know why that was never implemented
there (this hasn't been touched since the VCS initial import).

Samuel

Re: [PATCH] hurd: Add shared mig declarations

[Simon Marchi]

On 2020-05-31 3:20 a.m., Samuel Thibault wrote:
> Simon Marchi, le sam. 30 mai 2020 21:51:35 -0400, a ecrit:
>> On 2020-05-30 2:23 p.m., Samuel Thibault wrote:
>>> Fixes
>>>
>>> exc_request_S.c:177:24: error: no previous declaration for ‘exc_server’ [-Werror=missing-declarations]
>>>   177 | mig_external boolean_t exc_server
>>>
>>> gdb/ChangeLog:
>>>
>>> 	* config/i386/i386gnu.mn [%_S.o %_U.o] (COMPILE.post): Add
>>> 	"-include gnu-nat-mig.h".
>>> 	* gnu-nat-mig.h: New file.
>>> 	* gnu-nat.c: Include "gnu-nat-mig.h".
>>> 	(exc_server, msg_reply_server, notify_server,
>>> 	process_reply_server): Remove declarations.
>>
>> It took me a while to understand the underlying problem.  My understanding is that
>> gnu-nat.c calls this function exc_server, that is defined in the generated file.  The
>> generated file does not provide a header with declarations, so gnu-nat.c had its own
>> local declaration.  Since we now use the -Wmissing-declarations warning flag, and the
>> definition in the generated exc_request_S.c didn't see a corresponding declaration,
>> it caused that build failure.  Is that correct?  If so, please add that explanation
>> or equivalent to the commit log.
> 
> I have now added
> 
> “
> We are using -Werror=missing-declarations, and the _S.h files generated
> by mig do not currently include a declaration for the server routine.
> gnu-nat.c used to have its own external declarations, but better just
> share them between gnu-nat.c and the _S.c files.
> ”

Thanks, that sounds good.  And this way, I suppose that if for some reason the prototypes
don't match, we'll get a compilation error (which is a good thing).

>> My question now is: that MIG tool appears to generate both a header (%_S.h) and source
>> file (%_S.c) from defs files.  What is this header file used for, if it doesn't contain
>> the declaration for the functions in the source file?
> 
> Mig does include declarations for the functions of the .c files, but
> not for the server routine, I don't know why that was never implemented
> there (this hasn't been touched since the VCS initial import).

Ok.

Simon

Re: [PATCH] hurd: Add shared mig declarations

[Samuel Thibault]

Applied, thanks!

Samuel

Re: [PATCH] hurd: Add shared mig declarations

[Christian Biesinger]

On Sat, May 30, 2020 at 1:23 PM Samuel Thibault <samuel.thibault@gnu.org> wrote:
> Index: binutils-gdb/gdb/gnu-nat.c
> ===================================================================
> --- binutils-gdb.orig/gdb/gnu-nat.c
> +++ binutils-gdb/gdb/gnu-nat.c
> @@ -46,6 +46,9 @@ extern "C"
>  #include <hurd/sigpreempt.h>
>
>  #include <portinfo.h>
> +
> +/* MIG stubs are not yet ready for C++ compilation.  */
> +#include "gnu-nat-mig.h"
>  }

But since this is a header in GDB, why not just add that extern "C" to
make it ready for C++ compilation...?

Christian