machine specific patch (OpenBSD)

machine specific patch (OpenBSD)

[Manuel Giraud]

Hi,

I'd like to know if machine specific patches are something that can go
in the repo before officially releasing emacs 24 ? In my case, I'd like
to push some of the patches for OpenBSD^1. For instance, the following
patch avoid emacs 24 to freeze at start under X:

--8<---------------cut here---------------start------------->8---
=== modified file 'src/s/openbsd.h'
--- src/s/openbsd.h     2011-01-15 23:16:57 +0000
+++ src/s/openbsd.h     2011-07-13 13:28:30 +0000
@@ -3,3 +3,4 @@
 /* The same as NetBSD.  Note there are differences in configure.  */
 #include "netbsd.h"
 
+#define BROKEN_SIGIO
--8<---------------cut here---------------end--------------->8---

Best regards,

1: http://www.openbsd.org/cgi-bin/cvsweb/ports/editors/emacs23/patches/
-- 
Manuel Giraud

Re: machine specific patch (OpenBSD)

[Dan Nicolaescu]

Manuel Giraud <manuel.giraud@univ-nantes.fr> writes:

> Hi,
>
> I'd like to know if machine specific patches are something that can go
> in the repo before officially releasing emacs 24 ? In my case, I'd like
> to push some of the patches for OpenBSD^1. For instance, the following
> patch avoid emacs 24 to freeze at start under X:
>
>
> --8<---------------cut here---------------start------------->8---
> === modified file 'src/s/openbsd.h'
> --- src/s/openbsd.h     2011-01-15 23:16:57 +0000
> +++ src/s/openbsd.h     2011-07-13 13:28:30 +0000
> @@ -3,3 +3,4 @@
>  /* The same as NetBSD.  Note there are differences in configure.  */
>  #include "netbsd.h"
>  
> +#define BROKEN_SIGIO
> --8<---------------cut here---------------end--------------->8---
>
> Best regards,
>
> 1: http://www.openbsd.org/cgi-bin/cvsweb/ports/editors/emacs23/patches/

How about showing the patches and not a link to them ?
But before doing that please update the patches.  A quick look at one of
them showed that it defining macros that are no longer in use in the
tree.

Re: machine specific patch (OpenBSD)

[Manuel Giraud]

Dan Nicolaescu <dann@gnu.org> writes:

> How about showing the patches and not a link to them ?  But before
> doing that please update the patches.  A quick look at one of them
> showed that it defining macros that are no longer in use in the tree.

Ok, so far I've updated those:
--8<---------------cut here---------------start------------->8---
=== modified file 'src/minibuf.c'
--- src/minibuf.c       2011-06-24 21:25:22 +0000
+++ src/minibuf.c       2011-07-13 13:50:55 +0000
@@ -19,6 +19,7 @@
 
 
 #include <config.h>
+#include <errno.h>
 #include <stdio.h>
 #include <setjmp.h>
 
@@ -246,15 +247,19 @@
   size = 100;
   len = 0;
   line = (char *) xmalloc (size);
-  while ((s = fgets (line + len, size - len, stdin)) != NULL
-        && (len = strlen (line),
-            len == size - 1 && line[len - 1] != '\n'))
-    {
+ again:
+  if ((s = fgets (line + len, size - len, stdin)) != NULL) {
+    len = strlen (line);
+    if (len > 0 && line[len - 1] != '\n') {
       if (STRING_BYTES_BOUND / 2 < size)
        memory_full (SIZE_MAX);
       size *= 2;
       line = (char *) xrealloc (line, size);
+      goto again;
     }
+  } else if (errno == EINTR) {
+    goto again;
+  }
 
   if (s)
     {

=== modified file 'src/s/openbsd.h'
--- src/s/openbsd.h     2011-01-15 23:16:57 +0000
+++ src/s/openbsd.h     2011-07-13 13:28:30 +0000
@@ -3,3 +3,4 @@
 /* The same as NetBSD.  Note there are differences in configure.  */
 #include "netbsd.h"
 
+#define BROKEN_SIGIO

--8<---------------cut here---------------end--------------->8---

I didn't include others because they are related to mips64 and alpha
arch and i cannot test it.

-- 
Manuel Giraud

Re: machine specific patch (OpenBSD)

[Paul Eggert]

On 07/13/11 08:22, Manuel Giraud wrote:

> Ok, so far I've updated those:

<http://www.openbsd.org/cgi-bin/cvsweb/ports/editors/emacs23/patches/>
doesn't seem to be updated; it was last changed 3 months ago.  Are the
updated patches available anywhere other than in your email?

> I didn't include others because they are related to mips64 and alpha
> arch and i cannot test it.

Has anyone tested them?

The two mips64 files seem long enough that they'll require copyright
assignment papers.  Did you write them?  If not, who?

>> +#define BROKEN_SIGIO

Could you please explain why that's needed?  Is there some discussion
of this somewhere?  It'd be helpful to have that in a comment somewhere.

Regarding the minibuf.c patch:

Is a similar patch (for fgets) needed in xfaces.c's Fx_load_color_file
function, for the case where Emacs is configured without HAVE_X_WINDOWS?

The minibuf.c patch is a bit confusing, with gotos and suchlike.
Does the following patch fix the problem as well?  It's a longer patch,
but the resulting code should be more straightforward.

=== modified file 'src/minibuf.c'
--- src/minibuf.c	2011-06-24 21:25:22 +0000
+++ src/minibuf.c	2011-07-13 22:52:27 +0000
@@ -19,6 +19,7 @@ along with GNU Emacs.  If not, see <http
 
 
 #include <config.h>
+#include <errno.h>
 #include <stdio.h>
 #include <setjmp.h>
 
@@ -236,8 +237,9 @@ read_minibuf_noninteractive (Lisp_Object
 			     int allow_props, int inherit_input_method)
 {
   ptrdiff_t size, len;
-  char *line, *s;
+  char *line;
   Lisp_Object val;
+  int c;
 
   fprintf (stdout, "%s", SDATA (prompt));
   fflush (stdout);
@@ -246,22 +248,30 @@ read_minibuf_noninteractive (Lisp_Object
   size = 100;
   len = 0;
   line = (char *) xmalloc (size);
-  while ((s = fgets (line + len, size - len, stdin)) != NULL
-	 && (len = strlen (line),
-	     len == size - 1 && line[len - 1] != '\n'))
+
+  while ((c = getchar ()) != '\n')
     {
-      if (STRING_BYTES_BOUND / 2 < size)
-	memory_full (SIZE_MAX);
-      size *= 2;
-      line = (char *) xrealloc (line, size);
+      if (c < 0)
+	{
+	  if (errno != EINTR)
+	    break;
+	}
+      else
+	{
+	  if (len == size)
+	    {
+	      if (STRING_BYTES_BOUND / 2 < size)
+		memory_full (SIZE_MAX);
+	      size *= 2;
+	      line = (char *) xrealloc (line, size);
+	    }
+	  line[len++] = c;
+	}
     }
 
-  if (s)
+  if (len)
     {
-      char *nl = strchr (line, '\n');
-      if (nl)
-	*nl = '\0';
-      val = build_string (line);
+      val = make_string (line, len);
       xfree (line);
     }
   else

Re: machine specific patch (OpenBSD)

[Manuel Giraud]

Paul Eggert <eggert@cs.ucla.edu> writes:

> On 07/13/11 08:22, Manuel Giraud wrote:
>
>> Ok, so far I've updated those:
>
> <http://www.openbsd.org/cgi-bin/cvsweb/ports/editors/emacs23/patches/>
> doesn't seem to be updated; it was last changed 3 months ago.  Are the
> updated patches available anywhere other than in your email?

They are just in my email. The patches from the openbsd cvsweb are the
ones to make the last stable version of emacs (23.3) to work under
openbsd and make an installable package out of it.

I just like to have some stuff in the emacs repo in order to have emacs
24 eventually working "out of the box" on openbsd.

>> I didn't include others because they are related to mips64 and alpha
>> arch and i cannot test it.
>
> Has anyone tested them?

Not me, I don't have the hardware. jasper@ seems to have tested and
updated them.

> The two mips64 files seem long enough that they'll require copyright
> assignment papers.  Did you write them?  If not, who?

There is a FSF copyright, is it enough? I didn't write this file, it
seems to come frome here
<http://www.openbsd.org/cgi-bin/cvsweb/ports/editors/emacs21/patches/>
and is almost 6 years old.

>>> +#define BROKEN_SIGIO
>
> Could you please explain why that's needed?  Is there some discussion
> of this somewhere?  It'd be helpful to have that in a comment
> somewhere.

There was a discussion here:
http://marc.info/?l=openbsd-ports&m=128932473227826&w=2

and a solution found by Mike Belopuhov, here:
http://marc.info/?l=openbsd-ports&m=130019184729434&w=2

> Regarding the minibuf.c patch:
>
> Is a similar patch (for fgets) needed in xfaces.c's Fx_load_color_file
> function, for the case where Emacs is configured without HAVE_X_WINDOWS?
>
> The minibuf.c patch is a bit confusing, with gotos and suchlike.
> Does the following patch fix the problem as well?  It's a longer patch,
> but the resulting code should be more straightforward.

It works and I too prefer this one.

-- 
Manuel Giraud

Re: machine specific patch (OpenBSD)

[Paul Eggert]

Thanks for looking into it.  I committed the following into the Emacs
trunk as bzr 105287.  It addresses all the issues I saw, except for
the MIPS port.  The files for that don't look right: they
define a bunch of symbols that Emacs no longer uses.  The only symbol
that is current is VIRT_ADDR_VARIES, so maybe we'll have to fold
that in somehow, but that probably belongs in s/openbsd.h anyway.
Anyway, perhaps if you could get the person who does the MIPS port into
the discussion, so that they can explain what goes wrong on MIPS without
that patch, we could fix the MIPS port too.

======

Port to OpenBSD.
See http://lists.gnu.org/archive/html/emacs-devel/2011-07/msg00688.html
and the surrounding thread.
* minibuf.c (read_minibuf_noninteractive): Rewrite to use getchar
rather than fgets, and retry after EINTR.  Otherwise, 'emacs
--batch -f byte-compile-file' fails on OpenBSD if an inactivity
timer goes off.
* s/openbsd.h (BROKEN_SIGIO): Define.
* unexelf.c (unexec) [__OpenBSD__]:
Don't update the .mdebug section of the Alpha COFF symbol table.
=== modified file 'src/minibuf.c'
--- src/minibuf.c	2011-06-24 21:25:22 +0000
+++ src/minibuf.c	2011-07-19 15:46:18 +0000
@@ -19,6 +19,7 @@


 #include <config.h>
+#include <errno.h>
 #include <stdio.h>
 #include <setjmp.h>

@@ -236,8 +237,9 @@
 			     int allow_props, int inherit_input_method)
 {
   ptrdiff_t size, len;
-  char *line, *s;
+  char *line;
   Lisp_Object val;
+  int c;

   fprintf (stdout, "%s", SDATA (prompt));
   fflush (stdout);
@@ -246,22 +248,30 @@
   size = 100;
   len = 0;
   line = (char *) xmalloc (size);
-  while ((s = fgets (line + len, size - len, stdin)) != NULL
-	 && (len = strlen (line),
-	     len == size - 1 && line[len - 1] != '\n'))
+
+  while ((c = getchar ()) != '\n')
     {
-      if (STRING_BYTES_BOUND / 2 < size)
-	memory_full (SIZE_MAX);
-      size *= 2;
-      line = (char *) xrealloc (line, size);
+      if (c < 0)
+	{
+	  if (errno != EINTR)
+	    break;
+	}
+      else
+	{
+	  if (len == size)
+	    {
+	      if (STRING_BYTES_BOUND / 2 < size)
+		memory_full (SIZE_MAX);
+	      size *= 2;
+	      line = (char *) xrealloc (line, size);
+	    }
+	  line[len++] = c;
+	}
     }

-  if (s)
+  if (len)
     {
-      char *nl = strchr (line, '\n');
-      if (nl)
-	*nl = '\0';
-      val = build_string (line);
+      val = make_string (line, len);
       xfree (line);
     }
   else

=== modified file 'src/s/openbsd.h'
--- src/s/openbsd.h	2011-01-15 23:16:57 +0000
+++ src/s/openbsd.h	2011-07-19 17:19:32 +0000
@@ -1,5 +1,9 @@
 /* System file for openbsd.  */

-/* The same as NetBSD.  Note there are differences in configure.  */
+/* Nearly the same as NetBSD.  Note there are differences in configure.  */
 #include "netbsd.h"

+/* The symbol SIGIO is defined, but the feature doesn't work in the
+   way Emacs needs it to.  See
+   <http://article.gmane.org/gmane.os.openbsd.ports/46831>.  */
+#define BROKEN_SIGIO

=== modified file 'src/unexelf.c'
--- src/unexelf.c	2011-06-13 05:55:57 +0000
+++ src/unexelf.c	2011-07-19 16:47:05 +0000
@@ -1053,7 +1053,7 @@
       memcpy (NEW_SECTION_H (nn).sh_offset + new_base, src,
 	      NEW_SECTION_H (nn).sh_size);

-#ifdef __alpha__
+#if defined __alpha__ && !defined __OpenBSD__
       /* Update Alpha COFF symbol table: */
       if (strcmp (old_section_names + OLD_SECTION_H (n).sh_name, ".mdebug")
 	  == 0)
@@ -1072,7 +1072,7 @@
 	  symhdr->cbRfdOffset += new_data2_size;
 	  symhdr->cbExtOffset += new_data2_size;
 	}
-#endif /* __alpha__ */
+#endif /* __alpha__ && !__OpenBSD__ */

 #if defined (_SYSTYPE_SYSV)
       if (NEW_SECTION_H (nn).sh_type == SHT_MIPS_DEBUG

Re: machine specific patch (OpenBSD)

[Manuel Giraud]

Paul Eggert <eggert@cs.ucla.edu> writes:

> Thanks for looking into it.  I committed the following into the Emacs
> trunk as bzr 105287.  

Thanks for that. I've tested and the current Emacs trunk now compiles
(and works) without patch on OpenBSD/i386.

> [...]
> Anyway, perhaps if you could get the person who does the MIPS port into
> the discussion, so that they can explain what goes wrong on MIPS without
> that patch, we could fix the MIPS port too.

Ok, I'll ask on the ports mailing list to find out who is in charge of
the mips port.
-- 
Manuel Giraud