[comp.sources.bugs] bug in the 'ro' text formatter

rlacoste@kebra.laas.fr (Robert Lacoste) (02/26/90)

	I had a problem compiling the 'ro' text formatter submitted in
	comp.source.unix with the sun C compiler :

	Problem:
		In ro_macr.c, line 170, a call to strlen is made
		with an argument that can be NULL, giving a bus error
		on a SUN 3...

	Proposed correction:
		Replacement of all strlen calls by mystrlen, with the
		following definition:

			int mystrlen(s)
			char *s;
			{
			  if (s==NULL) return (0);
			  else return(strlen(s));
			}


	Any comment ?				Robert Lacoste.

jik@athena.mit.edu (Jonathan I. Kamens) (02/27/90)

In article <1126@laas.laas.fr>, rlacoste@kebra.laas.fr (Robert Lacoste) writes:
> 	Proposed correction:
> 		Replacement of all strlen calls by mystrlen, with the
> 		following definition:
> 
> 			int mystrlen(s)
> 			char *s;
> 			{
> 			  if (s==NULL) return (0);
> 			  else return(strlen(s));
> 			}

  I sure hope your compiler has inline functions and automatically knows
when to use them best; otherwise, you're introducing an awful lot of
overhead which is going to slow down the program quite a bit if there
are a lot of calls to strlen.

  Instead of what you've proposed, I suggest this alternative:

    #define mystrlen(s) ((s) ? strlen(s) : 0)

Jonathan Kamens			              USnail:
MIT Project Athena				11 Ashford Terrace
jik@Athena.MIT.EDU				Allston, MA  02134
Office: 617-253-8495			      Home: 617-782-0710

ag@amix.commodore.com (Keith Gabryelski) (02/28/90)

In article <1126@laas.laas.fr> rlacoste@kebra.laas.fr (Robert Lacoste) writes:
>	Problem:
>		In ro_macr.c, line 170, a call to strlen is made
>		with an argument that can be NULL, giving a bus error
>		on a SUN 3...
>
>	Proposed correction:
>		Replacement of all strlen calls by mystrlen, with the
>		following definition:
>
>			int mystrlen(s)
>			char *s;
>			{
>			  if (s==NULL) return (0);
>			  else return(strlen(s));
>			}
>

Hmmmmm....

>	Any comment ?

Yeah, add this to main():

	{
		int fd = open("/dev/zero", 0);
		mmap(0, 2048, 0x01, 0x11, fd, 0);
		close(fd)
	}

Or fix the null pointer bugs.

Pax, Keith

Ps, :-(
-- 
ag@amix.commodore.com        Keith Gabryelski          ...!cbmvax!amix!ag

jim@chimera.UUCP (Jim Levie) (03/03/90)

jik@athena.mit.edu (Jonathan I. Kamens) writes:

>In article <1126@laas.laas.fr>, rlacoste@kebra.laas.fr (Robert Lacoste) writes:
>> 	Proposed correction:
>> 		Replacement of all strlen calls by mystrlen, with the
>> 		following definition:
>> 
>> 			int mystrlen(s)
>> 			char *s;
>> 			{
>> 			  if (s==NULL) return (0);
>> 			  else return(strlen(s));
>> 			}

>... stuff deleted ...

>  Instead of what you've proposed, I suggest this alternative:

>    #define mystrlen(s) ((s) ? strlen(s) : 0)

Uh, you really don't want to do any of the above.  The problem is that ro is
trying to execute the ".ds" instruction, and for that it needs a place for
the string name and the string.  The posted code either used an existing
"insertion structure" (when the string register was being redefined), or
created a new "insertion structure" (with a NULL string pointer). It then
checked to see if the new string was larger than the old string, which was
fine as long as there was an old string, but which won't work when the string
register is being used for the first time (the string pointer is NULL).

I believe that the correct fix is something like the following code.  I
"indented" it so I could read it easier, so I can't do a diff.

From insert() in ro_macr.c

    /*
     * see if it is already defined
     */
    if((sptr = find2(name, slink))!=NULL)	
    {
	if(ro_verbose==TRUE)
	{
	    fprintf(stderr, "%cWarning: <%s> was defined to be <%s>\n",
		    BELL, name, sptr->mstr);
	    fprintf(stderr, "...now it is defined to be <%s>\n", tbuf);
	}
	/*
	 * allocate memory for the new string if it is longer 
	 * than the current string
	 */ 
	if(strlen(tbuf)>strlen(sptr->mstr))
	{
	    if((sptr->mstr = malloc((size_t)strlen(tbuf)+1))==NULL)
	    {
		fprintf(stderr,
			"Fatal: failed to allocate memory for string.\n");
		exit(-1);
	    }
	}
    }
    else 
    {
	if((sptr = (struct divfd *)malloc((size_t)sizeof(struct divfd)))==NULL)
	{
	    fprintf(stderr,
		    "Fatal: cannot allocate memory for insertion structure.\n"); 
	    exit(-1);
	}
	
	/*
	 * Allocation succeeded, set up links,  then allocate space for
	 * the string to be copied into
	 */
	sptr->prev = slink;		/* save previous link	*/
	slink = sptr;			/* reset link		*/
	strcpy(sptr->nm, name);
	sptr->mstr = NULL;
	if((sptr->mstr = malloc((size_t)strlen(tbuf)+1))==NULL)
	{
	    fprintf(stderr,
		    "Fatal: failed to allocate memory for string.\n");
	    exit(-1);
	}
    }
    
    /*
     * copy the string to the new memory
     */
    strcpy(sptr->mstr, tbuf);
}


-- 
=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+
 Jim Levie   REMTECH Inc  Huntsville, Al 
 The opinions expressed above are just that.
 Ph.    (205) 536-8581               email: uunet!ingr!chimera!jim