libguile/print.c fixes

libguile/print.c fixes

[Paul Jarc]

I'm working on a patch for libguile/print.c.  I intend to fix any bugs
that break comparison of foo with:
(with-input-from-string (format #f "~S" foo) read)
A comment says that purely numeric symbol names and symbol names
starting with ' are broken, so I'm working on those.  I think strings
and symbol names containing control characters (is it ok to depend on
ASCII?) could be improved by escaping; some cases seem to be broken.
Symbol names containing \ don't strictly need to be printed in
#{this}# style, but it would significantly simplify the code if they
were.  Any objections?  Any other cases that need to be fixed?

I haven't done paperwork for Guile yet.  Could someone send me the
current version of request-disclaim.future?


paul


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/guile-devel

Re: libguile/print.c fixes

[Rob Browning]

prj@po.cwru.edu (Paul Jarc) writes:

> I haven't done paperwork for Guile yet.  Could someone send me the
> current version of request-disclaim.future?

Sure, I'd be happy to if someone can remind me where to find it.  ISTR
it's in a dir on one of the gnu servers...

-- 
Rob Browning
rlb @defaultvalue.org, @linuxdevel.com, and @debian.org
Previously @cs.utexas.edu
GPG starting 2002-11-03 = 14DD 432F AE39 534D B592  F9A0 25C8 D377 8C7E 73A4


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/guile-devel

Re: libguile/print.c fixes

[Marius Vollmer]

Rob Browning <rlb@defaultvalue.org> writes:

> prj@po.cwru.edu (Paul Jarc) writes:
> 
> > I haven't done paperwork for Guile yet.  Could someone send me the
> > current version of request-disclaim.future?
> 
> Sure, I'd be happy to if someone can remind me where to find it.  ISTR
> it's in a dir on one of the gnu servers...

It's in fencepost.gnu.org:/gd/gnuorg/request-assign.future.

Here it is:





Please email the following information to assign@gnu.org, and we
will send you the assignment form for your past and future changes.

Please use your full legal name (in ASCII characters) as the subject
line of the message.
----------------------------------------------------------------------
REQUEST: SEND FORM FOR PAST AND FUTURE CHANGES

[What is the name of the program or package you're contributing to?]


[Did you copy any files or text written by someone else in these changes?
Even if that material is free software, we need to know about it.]


[Do you have an employer who might have a basis to claim to own
your changes?  Do you attend a school which might make such a claim?]


[For the copyright registration, what country are you a citizen of?]


[What year were you born?]


[Please write your email address here.]


[Please write your postal address here.]





[Which files have you changed so far, and which new files have you written
so far?]







-- 
GPG: D5D4E405 - 2F9B BCCC 8527 692A 04E3  331E FAF8 226A D5D4 E405


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/guile-devel

Re: libguile/print.c fixes

[Rob Browning]

prj@po.cwru.edu (Paul Jarc) writes:

> Symbol names containing \ don't strictly need to be printed in
> #{this}# style, but it would significantly simplify the code if they
> were.  Any objections?

Not sure.  It's nice to avoid the escaping when you don't need it, but
I'm not sure how much complexity that introduces.

If we have to scan the symbol at construction time anyway, I wonder if
there's any clever way we could just set a flag (bit) that says
whether or not the symbol name needs escape encoding when we create
it?  Do we have any unused symbol bits lying around?

-- 
Rob Browning
rlb @defaultvalue.org, @linuxdevel.com, and @debian.org
Previously @cs.utexas.edu
GPG starting 2002-11-03 = 14DD 432F AE39 534D B592  F9A0 25C8 D377 8C7E 73A4


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/guile-devel

Re: libguile/print.c fixes

[Paul Jarc]

[-- Attachment #1: Type: text/plain, Size: 678 bytes --]

Rob Browning <rlb@defaultvalue.org> wrote:
> prj@po.cwru.edu (Paul Jarc) writes:
>> Symbol names containing \ don't strictly need to be printed in
>> #{this}# style, but it would significantly simplify the code if they
>> were.  Any objections?
>
> Not sure.  It's nice to avoid the escaping when you don't need it, but
> I'm not sure how much complexity that introduces.

I guess it's not that complex, although it does introduce a
performance hit for certain long symbol names, since part of the
string is re-scanned.  This patch fixes the bugs with symbol names,
but leaves backslash handling the way it is.
	* print.c (scm_print_symbol_name): Bug fixes and comments.


paul

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: guile.patch --]
[-- Type: text/x-patch, Size: 3046 bytes --]

Index: libguile/print.c
===================================================================
RCS file: /cvsroot/guile/guile/guile-core/libguile/print.c,v
retrieving revision 1.149
diff -u -r1.149 print.c
--- libguile/print.c	5 Apr 2003 20:45:17 -0000	1.149
+++ libguile/print.c	6 May 2003 14:48:58 -0000
@@ -282,27 +282,37 @@
 void
 scm_print_symbol_name (const char *str, size_t len, SCM port)
 {
-  size_t pos;
+  /* This points to the first character that has not yet been written to the
+   * port. */
+  size_t pos = 0;
+  /* This points to the character we're currently looking at. */
   size_t end;
-  int weird;
-  int maybe_weird;
+  /* If the name contains weird characters, we'll escape them with
+   * backslashes and set this flag; it indicates that we should surround the
+   * name with "#{" and "}#". */
+  int weird = 0;
+  /* Backslashes are not sufficient to make a name weird, but if a name is
+   * weird because of other characters, backslahes need to be escaped too.
+   * The first time we see a backslash, we set maybe_weird, and mw_pos points
+   * to the backslash.  Then if the name turns out to be weird, we re-process
+   * everything starting from mw_pos. */
+  int maybe_weird = 0;
   size_t mw_pos = 0;
-  
-  pos = 0;
-  weird = 0;
-  maybe_weird = 0;
-  
-  /* XXX - Lots of weird symbol names are missed, such as "12" or
-     "'a". */
+  /* If the name is purely numeric, then it's weird as a whole, even though
+   * none of the individual characters is weird.  But we won't know this
+   * until we reach the end of the name.  This flag describes the part of the
+   * name we've looked at so far. */
+  int all_digits = 1;
 
   if (len == 0)
     scm_lfwrite ("#{}#", 4, port);
-  else if (str[0] == '#' || str[0] == ':' || str[len-1] == ':')
+  else if (str[0] == '#' || str[0] == '\'' ||
+	   str[0] == ':' || str[len-1] == ':')
     {
       scm_lfwrite ("#{", 2, port);
       weird = 1;
     }
-  
+
   for (end = pos; end < len; ++end)
     switch (str[end])
       {
@@ -314,8 +324,10 @@
       case ')':
       case '"':
       case ';':
+      case '#':
       case SCM_WHITE_SPACES:
       case SCM_LINE_INCREMENTORS:
+	all_digits = 0;
       weird_handler:
 	if (maybe_weird)
 	  {
@@ -328,9 +340,7 @@
 	    weird = 1;
 	  }
 	if (pos < end)
-	  {
-	    scm_lfwrite (str + pos, end - pos, port);
-	  }
+	  scm_lfwrite (str + pos, end - pos, port);
 	{
 	  char buf[2];
 	  buf[0] = '\\';
@@ -340,6 +350,7 @@
 	pos = end + 1;
 	break;
       case '\\':
+	all_digits = 0;
 	if (weird)
 	  goto weird_handler;
 	if (!maybe_weird)
@@ -348,14 +359,18 @@
 	    mw_pos = pos;
 	  }
 	break;
-      case '}':
-      case '#':
-	if (weird)
-	  goto weird_handler;
+      case '0': case '1': case '2': case '3': case '4':
+      case '5': case '6': case '7': case '8': case '9':
 	break;
       default:
+	all_digits = 0;
 	break;
       }
+  if (all_digits)
+    {
+      scm_lfwrite ("#{", 2, port);
+      weird = 1;
+    }
   if (pos < end)
     scm_lfwrite (str + pos, end - pos, port);
   if (weird)

[-- Attachment #3: Type: text/plain, Size: 142 bytes --]

_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/guile-devel

Re: libguile/print.c fixes

[Marius Vollmer]

prj@po.cwru.edu (Paul Jarc) writes:

> 	* print.c (scm_print_symbol_name): Bug fixes and comments.

When writing a ChangeLog entry, please try to be more specific about
what bugs have been fixed how.  When a new function has been added,
then it is OK to just say "new function" but when existing code is
changed, it is good to see why it was changed, and how.

-- 
GPG: D5D4E405 - 2F9B BCCC 8527 692A 04E3  331E FAF8 226A D5D4 E405


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/guile-devel

Re: libguile/print.c fixes

[Paul Jarc]

Marius Vollmer <mvo@zagadka.de> wrote:
> prj@po.cwru.edu (Paul Jarc) writes:
>> 	* print.c (scm_print_symbol_name): Bug fixes and comments.
>
> When writing a ChangeLog entry, please try to be more specific about
> what bugs have been fixed how.

Right - I've already sent a better ChangeLog entry to Rob off-list.
I've also submitted my paperwork request.  I'll post again when it's
done.


paul


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/guile-devel

Re: libguile/print.c fixes

[Marius Vollmer]

prj@po.cwru.edu (Paul Jarc) writes:

> Marius Vollmer <mvo@zagadka.de> wrote:
> > prj@po.cwru.edu (Paul Jarc) writes:
> >> 	* print.c (scm_print_symbol_name): Bug fixes and comments.
> >
> > When writing a ChangeLog entry, please try to be more specific about
> > what bugs have been fixed how.
> 
> Right - I've already sent a better ChangeLog entry to Rob off-list.
> I've also submitted my paperwork request.  I'll post again when it's
> done.

Excellent!  Thanks a lot!


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/guile-devel

Re: libguile/print.c fixes

[Paul Jarc]

I wrote:
> I've also submitted my paperwork request.  I'll post again when it's
> done.

My copyright paperwork for Guile is now done.  Rob, can you install my
setgroups and symbol-printing patches?


paul


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/guile-devel

Re: libguile/print.c fixes

[Paul Jarc]

[-- Attachment #1: Type: text/plain, Size: 386 bytes --]

I wrote:
> My copyright paperwork for Guile is now done.  Rob, can you install my
> setgroups and symbol-printing patches?

In case someone else is less busy... top-level ChangeLog:
	* configure.in: check for setgroups.
libguile/ChangeLog:
	* posix.c (scm_setgroups): new function.
	* print.c (scm_print_symbol_name): add comments and fix bugs
	for #{1}#, #{'x}#, and #{}\#\ }#.


paul

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: setgroups.patch --]
[-- Type: text/x-patch, Size: 2202 bytes --]

Index: configure.in
===================================================================
RCS file: /cvsroot/guile/guile/guile-core/configure.in,v
retrieving revision 1.222
diff -u -r1.222 configure.in
--- configure.in	21 Jun 2003 00:15:09 -0000	1.222
+++ configure.in	30 Jun 2003 22:32:37 -0000
@@ -591,7 +591,7 @@
     [Define if the system supports Unix-domain (file-domain) sockets.])
 fi
 
-AC_CHECK_FUNCS(socketpair getgroups setpwent pause tzset)
+AC_CHECK_FUNCS(socketpair getgroups setgroups setpwent pause tzset)
 
 AC_CHECK_FUNCS(sethostent   gethostent   endhostent   dnl
                setnetent    getnetent    endnetent    dnl
Index: libguile/posix.c
===================================================================
RCS file: /cvsroot/guile/guile/guile-core/libguile/posix.c,v
retrieving revision 1.118
diff -u -r1.118 posix.c
--- libguile/posix.c	14 Jun 2003 05:36:02 -0000	1.118
+++ libguile/posix.c	30 Jun 2003 22:32:38 -0000
@@ -228,6 +228,46 @@
 #undef FUNC_NAME  
 #endif
 
+#ifdef HAVE_SETGROUPS
+SCM_DEFINE (scm_setgroups, "setgroups", 1, 0, 0,
+            (SCM group_vec),
+	    "Set the supplementary group IDs to those found in the vector argument.")
+#define FUNC_NAME s_scm_setgroups
+{
+  size_t ngroups;
+  size_t size;
+  size_t i;
+  int result;
+  int save_errno;
+  GETGROUPS_T *groups;
+
+  SCM_VALIDATE_VECTOR (SCM_ARG1, group_vec);
+
+  ngroups = SCM_VECTOR_LENGTH (group_vec);
+
+  /* validate before allocating, so we don't have to worry about leaks */
+  for (i = 0; i < ngroups; i++)
+    SCM_VALIDATE_INUM (0, SCM_VECTOR_REF (group_vec, i));
+
+  size = ngroups * sizeof (GETGROUPS_T);
+  /* XXX - if (size / sizeof (GETGROUPS_T) != ngroups) out-of-range */
+  groups = scm_malloc (size);
+  if (groups == NULL)
+    SCM_MEMORY_ERROR;
+  for(i = 0; i < ngroups; i++)
+    groups [i] = SCM_INUM (SCM_VECTOR_REF (group_vec, i));
+
+  result = setgroups (ngroups, groups);
+  save_errno = errno; /* don't let free() touch errno */
+  free (groups);
+  errno = save_errno;
+  if (result < 0)
+    SCM_SYSERROR;
+  return SCM_UNSPECIFIED;
+}
+#undef FUNC_NAME
+#endif
+
 #ifdef HAVE_GETPWENT
 SCM_DEFINE (scm_getpwuid, "getpw", 0, 1, 0,
             (SCM user),

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: print_symbol.patch --]
[-- Type: text/x-patch, Size: 3033 bytes --]

Index: libguile/print.c
===================================================================
RCS file: /cvsroot/guile/guile/guile-core/libguile/print.c,v
retrieving revision 1.150
diff -u -r1.150 print.c
--- libguile/print.c	12 May 2003 20:46:52 -0000	1.150
+++ libguile/print.c	30 Jun 2003 22:32:38 -0000
@@ -300,27 +300,34 @@
 void
 scm_print_symbol_name (const char *str, size_t len, SCM port)
 {
-  size_t pos;
+  /* This points to the first character that has not yet been written to the
+   * port. */
+  size_t pos = 0;
+  /* This points to the character we're currently looking at. */
   size_t end;
-  int weird;
-  int maybe_weird;
+  /* If the name contains weird characters, we'll escape them with
+   * backslashes and set this flag; it indicates that we should surround the
+   * name with "#{" and "}#". */
+  int weird = 0;
+  /* Backslashes are not sufficient to make a name weird, but if a name is
+   * weird because of other characters, backslahes need to be escaped too.
+   * The first time we see a backslash, we set maybe_weird, and mw_pos points
+   * to the backslash.  Then if the name turns out to be weird, we re-process
+   * everything starting from mw_pos. */
+  int maybe_weird = 0;
   size_t mw_pos = 0;
-  
-  pos = 0;
-  weird = 0;
-  maybe_weird = 0;
-  
-  /* XXX - Lots of weird symbol names are missed, such as "12" or
-     "'a". */
+  /* If the name is purely numeric, then it's weird as a whole, even though
+   * none of the individual characters is weird.  But we won't know this
+   * until we reach the end of the name.  This flag describes the part of the
+   * name we've looked at so far. */
+  int all_digits = 1;
 
-  if (len == 0)
-    scm_lfwrite ("#{}#", 4, port);
-  else if (str[0] == '#' || str[0] == ':' || str[len-1] == ':')
+  if (len == 0 || str[0] == '\'' || str[0] == ':' || str[len-1] == ':')
     {
       scm_lfwrite ("#{", 2, port);
       weird = 1;
     }
-  
+
   for (end = pos; end < len; ++end)
     switch (str[end])
       {
@@ -332,8 +339,10 @@
       case ')':
       case '"':
       case ';':
+      case '#':
       case SCM_WHITE_SPACES:
       case SCM_LINE_INCREMENTORS:
+	all_digits = 0;
       weird_handler:
 	if (maybe_weird)
 	  {
@@ -346,9 +355,7 @@
 	    weird = 1;
 	  }
 	if (pos < end)
-	  {
-	    scm_lfwrite (str + pos, end - pos, port);
-	  }
+	  scm_lfwrite (str + pos, end - pos, port);
 	{
 	  char buf[2];
 	  buf[0] = '\\';
@@ -358,6 +365,7 @@
 	pos = end + 1;
 	break;
       case '\\':
+	all_digits = 0;
 	if (weird)
 	  goto weird_handler;
 	if (!maybe_weird)
@@ -366,14 +374,18 @@
 	    mw_pos = pos;
 	  }
 	break;
-      case '}':
-      case '#':
-	if (weird)
-	  goto weird_handler;
+      case '0': case '1': case '2': case '3': case '4':
+      case '5': case '6': case '7': case '8': case '9':
 	break;
       default:
+	all_digits = 0;
 	break;
       }
+  if (all_digits)
+    {
+      scm_lfwrite ("#{", 2, port);
+      weird = 1;
+    }
   if (pos < end)
     scm_lfwrite (str + pos, end - pos, port);
   if (weird)

[-- Attachment #4: Type: text/plain, Size: 142 bytes --]

_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/guile-devel

patch ping (was: libguile/print.c fixes)

[Paul Jarc]

Any feedback one these two patches?  One is a bugfix; the other is a
straightforward new POSIX function.  My paperwork is done, and I can
repost the patches if needed.

I wrote:
> In case someone else is less busy... top-level ChangeLog:
> 	* configure.in: check for setgroups.
> libguile/ChangeLog:
> 	* posix.c (scm_setgroups): new function.
> 	* print.c (scm_print_symbol_name): add comments and fix bugs
> 	for #{1}#, #{'x}#, and #{}\#\ }#.


paul


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/guile-devel

Re: patch ping (was: libguile/print.c fixes)

[Marius Vollmer]

prj@po.cwru.edu (Paul Jarc) writes:

> Any feedback one these two patches?

Next weekend?  Sorry, I'm in batch mode with my thesis...

-- 
GPG: D5D4E405 - 2F9B BCCC 8527 692A 04E3  331E FAF8 226A D5D4 E405


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/guile-devel

Re: patch ping

[Paul Jarc]

Marius Vollmer <mvo@zagadka.de> wrote:
> prj@po.cwru.edu (Paul Jarc) writes:
>> Any feedback one these two patches?
>
> Next weekend?  Sorry, I'm in batch mode with my thesis...

No problem.  I was just curious, since no one had said anything.


paul


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/guile-devel

Re: patch ping

[Marius Vollmer]

prj@po.cwru.edu (Paul Jarc) writes:

> Marius Vollmer <mvo@zagadka.de> wrote:
> > prj@po.cwru.edu (Paul Jarc) writes:
> >> Any feedback one these two patches?
> >
> > Next weekend?  Sorry, I'm in batch mode with my thesis...
> 
> No problem.  I was just curious, since no one had said anything.

Ok, I have applied it.  Thanks!  I tweaked so setgroups function
slightly so that it is hopefully totally type correct.  Your version
worked only with gids that fit into a fixnum (30 bits) and there might
32 bit gids out there.

-- 
GPG: D5D4E405 - 2F9B BCCC 8527 692A 04E3  331E FAF8 226A D5D4 E405


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/guile-devel

Re: patch ping

[Paul Jarc]

Marius Vollmer <mvo@zagadka.de> wrote:
> I tweaked so setgroups function slightly so that it is hopefully
> totally type correct.  Your version worked only with gids that fit
> into a fixnum (30 bits) and there might 32 bit gids out there.

Thanks, that looks good.  Here's a further patch to take care of the
XXX comment.  Maybe it's overly paranoid, but I figure it can't hurt.

	* posix.c (scm_setgroups): Check that the gid list is not too
	long.

Index: libguile/posix.c
===================================================================
RCS file: /cvsroot/guile/guile/guile-core/libguile/posix.c,v
retrieving revision 1.119
diff -u -r1.119 posix.c
--- libguile/posix.c	27 Jul 2003 16:20:21 -0000	1.119
+++ libguile/posix.c	28 Jul 2003 16:35:01 -0000
@@ -253,7 +253,8 @@
     }
 
   size = ngroups * sizeof (GETGROUPS_T);
-  /* XXX - if (size / sizeof (GETGROUPS_T) != ngroups) out-of-range */
+  if (size / sizeof (GETGROUPS_T) != ngroups)
+    SCM_OUT_OF_RANGE (SCM_ARG1, SCM_MAKINUM (ngroups));
   groups = scm_malloc (size);
   for(i = 0; i < ngroups; i++)
     groups [i] = SCM_NUM2ULONG (1, SCM_VECTOR_REF (group_vec, i));


paul


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/guile-devel

Re: patch ping

[Marius Vollmer]

prj@po.cwru.edu (Paul Jarc) writes:

> Here's a further patch to take care of the
> XXX comment.  Maybe it's overly paranoid, but I figure it can't hurt.

Yep, applied.  Thanks!

-- 
GPG: D5D4E405 - 2F9B BCCC 8527 692A 04E3  331E FAF8 226A D5D4 E405


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/guile-devel