Re: Willing to debug bug #3542 (23.0.94; File access via UNC path slow again under Windows)

[Eli Zaretskii <eliz@gnu.org>]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: schwab@linux-m68k.org,  emacs-devel@gnu.org
> Date: Tue, 14 Jul 2009 14:18:53 -0400
> 
> >   directory_nbytes = SBYTES (directory);
> >   if (directory_nbytes == 0
> >       || !IS_ANY_SEP (SREF (directory, directory_nbytes - 1)))
> >     needsep = 1;
> >   [...]
> > 		  int nbytes = len + directory_nbytes + needsep;
> > 		  fullname = make_uninit_multibyte_string (nbytes, nbytes);
> > 		  bcopy (SDATA (directory), SDATA (fullname),
> > 			 directory_nbytes);
> 
> make_uninit_multibyte_string calls allocate_string_data which does
> 
>   STRING_DATA (s)[nbytes] = '\0';
> 
> so the destination of the `bcopy' already has the terminating NUL.

Perhaps most of the places where we use these paradigms are okay due
to all these subtle corners that together make everything work.  But
IMHO it's inherently unsafe to use character arrays that are not true
C strings as if they were C strings.  For one, it violates the mental
model each C programmer has about strings, and that can easily lead to
misunderstanding, confusion, and bugs.  For example (from dbusbind.c):

  char x[DBUS_MAXIMUM_SIGNATURE_LENGTH];
  [...]
	strcpy (x, SDATA (CAR_SAFE (XD_NEXT_VALUE (elt))));
  [...]
      sprintf (signature, "%c%s", dtype, x);

or

      case DBUS_TYPE_SIGNATURE:
	{
	  char *val = SDATA (Fstring_make_unibyte (object));
	  XD_DEBUG_MESSAGE ("%c %s", dtype, val);

How can one convince herself that this code is safe without knowing
too much about the Lisp strings whose data gets handled here as C
strings?  Can they have embedded nulls or cannot they?

Same here (editfns.c):

      if (SBYTES (val) > message_length)
	{
	  message_length = SBYTES (val);
	  message_text = (char *)xrealloc (message_text, message_length);
	}
      bcopy (SDATA (val), message_text, SBYTES (val));
      message2 (message_text, SBYTES (val),
		STRING_MULTIBYTE (val));

message_text[] is not a C string here, because it's not
null-terminated (and doesn't have enough space to be terminated).
Without looking at the implementation of message2, whose 1st arg is a
`char *', how can one know that there's no bug here?

Or here (search.c):

	  raw_pattern_size = SCHARS (string);
	  raw_pattern_size_byte = SCHARS (string);
	  raw_pattern = (unsigned char *) alloca (raw_pattern_size + 1);
	  copy_text (SDATA (string), raw_pattern,
		     SBYTES (string), 1, 0);

raw_pattern[] is not null-terminated, and we then use it, directly and
indirectly, in many places.  Without studying each use, there's no way
you can determine that there cannot be a bug here.

Etc., etc. -- I see other places where maybe it works, maybe it
doesn't.  One needs to study the code very carefully and look at many
functions up and down the call stack, just to determine if a few lines
don't constitute a bug.