fputs() and puts() now share the same common code. Added abort checks in order to avoid turning the memcpy() operations into an uninterruptable sequence.

2025-12-08 14:59:05 +00:00 · 2023-09-06 13:24:15 +02:00
parent 45d118101a
commit a7389454bb
2 changed files with 135 additions and 109 deletions
--- a/library/stdio_fputs.c
+++ b/library/stdio_fputs.c
@@ -41,12 +41,14 @@

 /****************************************************************************/

+/* Both fputs() and puts() share this function. */
 int
-fputs(const char *s, FILE *stream)
+__fputs(const char *s, int line_feed, FILE *stream)
 {
 	struct iob * file = (struct iob *)stream;
 	size_t total_size;
 	int result = EOF;
+	int buffer_mode;
 	int c;

 	ENTER();
@@ -56,12 +58,12 @@ fputs(const char *s, FILE *stream)

 	assert( s != NULL && stream != NULL );

-	if(__check_abort_enabled)
+	if (__check_abort_enabled)
 		__check_abort();

 	#if defined(CHECK_FOR_NULL_POINTERS)
 	{
-		if(s == NULL || stream == NULL)
+		if (s == NULL || stream == NULL)
 		{
 			__set_errno(EFAULT);
 			goto out;
@@ -75,80 +77,104 @@ fputs(const char *s, FILE *stream)
 	assert( FLAG_IS_SET(file->iob_Flags,IOBF_IN_USE) );
 	assert( file->iob_BufferSize > 0 );

-	if(__fputc_check(stream) < 0)
+	if (__fputc_check(stream) < 0)
 		goto out;

-	total_size = strlen(s);
-	if(total_size > 0)
-	{
-		int buffer_mode;
+	assert( 0 <= file->iob_BufferWriteBytes );
+	assert( file->iob_BufferWriteBytes <= file->iob_BufferSize );
+	assert( file->iob_BufferPosition <= file->iob_BufferSize );

 	/* We perform line buffering to improve readability of the
-		   buffered text if buffering was disabled and the output
-		   goes to an interactive stream. */
+	 * buffered text if buffering was disabled and the output
+	 * goes to an interactive stream.
+	 */
 	buffer_mode = (file->iob_Flags & IOBF_BUFFER_MODE);
-		if(buffer_mode == IOBF_BUFFER_MODE_NONE)
+	if (buffer_mode == IOBF_BUFFER_MODE_NONE)
 	{
 		struct fd * fd = __fd[file->iob_Descriptor];

 		__fd_lock(fd);

-			if(FLAG_IS_SET(fd->fd_Flags,FDF_IS_INTERACTIVE))
+		if (FLAG_IS_SET(fd->fd_Flags, FDF_IS_INTERACTIVE))
 			buffer_mode = IOBF_BUFFER_MODE_LINE;

 		__fd_unlock(fd);
 	}

+	total_size = strlen(s);
+	if (total_size > 0)
+	{
 		if (buffer_mode == IOBF_BUFFER_MODE_LINE)
 		{
-			while(total_size > 0)
+			while (total_size > 0)
 			{
 				/* Is there still room in the write buffer to store
-				   more of the string? */
-				if(file->iob_BufferWriteBytes < file->iob_BufferSize)
+				 * more of the string?
+				 */
+				if (file->iob_BufferWriteBytes < file->iob_BufferSize)
 				{
 					unsigned char * buffer = &file->iob_Buffer[file->iob_BufferWriteBytes];
 					size_t num_buffer_bytes;
 					const char * lf;

+					/* Give the user a chance to abort what could otherwise
+					 * become an uninterrupted series of copying operations.
+					 */
+					if (__check_abort_enabled)
+						__check_abort();
+
 					/* Store only as many characters as will fit into the write buffer. */
+					assert( file->iob_BufferSize >= file->iob_BufferWriteBytes );
+
 					num_buffer_bytes = file->iob_BufferSize - file->iob_BufferWriteBytes;
-					if(total_size < num_buffer_bytes)
+					if (total_size < num_buffer_bytes)
 						num_buffer_bytes = total_size;

 					/* Try to find a line feed in the string. If there is one,
-					   reduce the number of characters to write to the sequence
-					   which ends with the line feed character. */
+					 * reduce the number of characters to write to the sequence
+					 * which ends with the line feed character.
+					 */
 					lf = memchr(s, '\n', num_buffer_bytes);
-					if(lf != NULL)
+					if (lf != NULL)
+					{
+						assert( (size_t)(lf + 1 - s) <= num_buffer_bytes );
+
 						num_buffer_bytes = lf + 1 - s;
+					}
+
+					assert( num_buffer_bytes > 0 );

 					memmove(buffer, s, num_buffer_bytes);
 					s += num_buffer_bytes;

+					assert( file->iob_BufferWriteBytes + num_buffer_bytes <= file->iob_BufferSize );
+
 					file->iob_BufferWriteBytes += num_buffer_bytes;

 					/* Write the buffer to disk if it's full or contains a line feed. */
-					if((lf != NULL || __iob_write_buffer_is_full(file)) && __flush_iob_write_buffer(file) < 0)
+					if ((lf != NULL || __iob_write_buffer_is_full(file)) && __flush_iob_write_buffer(file) < 0)
 					{
 						/* Abort with error. */
 						goto out;
 					}

 					/* Stop as soon as no further data needs to be written. */
+					assert( total_size >= num_buffer_bytes );
+
 					total_size -= num_buffer_bytes;
-					if(total_size == 0)
+					if (total_size == 0)
 						break;

 					/* If there is again room in the output buffer,
-					   repeat this optimization. */
-					if(file->iob_BufferWriteBytes < file->iob_BufferSize)
+					 * repeat this optimization.
+					 */
+					if (file->iob_BufferWriteBytes < file->iob_BufferSize)
 						continue;
 				}

 				c = (*s++);

-				if(__putc_line_buffered(c,(FILE *)file) == EOF)
+				if (__putc_line_buffered(c, (FILE *)file) == EOF)
 					goto out;

 				total_size--;
@@ -160,28 +186,29 @@ fputs(const char *s, FILE *stream)

 			/* We bypass the buffer entirely. */
 			num_bytes_written = write(file->iob_Descriptor, s, total_size);
-			if(num_bytes_written == -1)
+			if (num_bytes_written == -1)
 			{
-				SET_FLAG(file->iob_Flags,IOBF_ERROR);
+				SET_FLAG(file->iob_Flags, IOBF_ERROR);
 				goto out;
 			}
 		}
 		else
 		{
-			while(total_size > 0)
+			while (total_size > 0)
 			{
 				/* If there is more data to be written than the write buffer will hold
-				   and the write buffer is empty anyway, then we'll bypass the write
-				   buffer entirely. */
-				if(file->iob_BufferWriteBytes == 0 && total_size >= (size_t)file->iob_BufferSize)
+				 * and the write buffer is empty anyway, then we'll bypass the write
+				 * buffer entirely.
+				 */
+				if (file->iob_BufferWriteBytes == 0 && total_size >= (size_t)file->iob_BufferSize)
 				{
 					ssize_t num_bytes_written;

 					/* We bypass the buffer entirely. */
 					num_bytes_written = write(file->iob_Descriptor, s, total_size);
-					if(num_bytes_written == -1)
+					if (num_bytes_written == -1)
 					{
-						SET_FLAG(file->iob_Flags,IOBF_ERROR);
+						SET_FLAG(file->iob_Flags, IOBF_ERROR);
 						goto out;
 					}

@@ -189,43 +216,59 @@ fputs(const char *s, FILE *stream)
 				}

 				/* Is there still room in the write buffer to store
-				   more of the string? */
-				if(file->iob_BufferWriteBytes < file->iob_BufferSize)
+				 * more of the string?
+				 */
+				if (file->iob_BufferWriteBytes < file->iob_BufferSize)
 				{
 					unsigned char * buffer = &file->iob_Buffer[file->iob_BufferWriteBytes];
 					size_t num_buffer_bytes;

+					/* Give the user a chance to abort what could otherwise
+					 * become an uninterrupted series of copying operations.
+					 */
+					if (__check_abort_enabled)
+						__check_abort();
+
 					/* Store only as many characters as will fit into the write buffer. */
+					assert( file->iob_BufferSize >= file->iob_BufferWriteBytes );
+
 					num_buffer_bytes = file->iob_BufferSize - file->iob_BufferWriteBytes;
-					if(total_size < num_buffer_bytes)
+					if (total_size < num_buffer_bytes)
 						num_buffer_bytes = total_size;

+					assert( num_buffer_bytes > 0 );
+
 					memmove(buffer, s, num_buffer_bytes);
 					s += num_buffer_bytes;

+					assert( file->iob_BufferWriteBytes + num_buffer_bytes <= file->iob_BufferSize );
+
 					file->iob_BufferWriteBytes += num_buffer_bytes;

 					/* Write a full buffer to disk. */
-					if(__iob_write_buffer_is_full(file) && __flush_iob_write_buffer(file) < 0)
+					if (__iob_write_buffer_is_full(file) && __flush_iob_write_buffer(file) < 0)
 					{
 						/* Abort with error. */
 						goto out;
 					}

 					/* Stop as soon as no further data needs to be written. */
+					assert( total_size >= num_buffer_bytes );
+
 					total_size -= num_buffer_bytes;
-					if(total_size == 0)
+					if (total_size == 0)
 						break;

 					/* If there is again room in the output buffer,
-					   try this optimization again. */
-					if(file->iob_BufferWriteBytes < file->iob_BufferSize)
+					 * try this optimization again.
+					 */
+					if (file->iob_BufferWriteBytes < file->iob_BufferSize)
 						continue;
 				}

 				c = (*s++);

-				if(__putc_fully_buffered(c,(FILE *)file) == EOF)
+				if (__putc_fully_buffered(c, (FILE *)file) == EOF)
 					goto out;

 				total_size--;
@@ -233,26 +276,63 @@ fputs(const char *s, FILE *stream)
 		}
 	}

+	if (line_feed != 0 && __putc(line_feed, stream, buffer_mode) == EOF)
+		goto out;
+
 	result = OK;

 out:

 	/* Note: if buffering is disabled for this stream, then we still
-	   may have buffered data around, queued to be printed right now.
-	   This is intended to improve performance as it takes more effort
-	   to write a single character to a file than to write a bunch. */
-	if(result == 0 && (file->iob_Flags & IOBF_BUFFER_MODE) == IOBF_BUFFER_MODE_NONE)
+	 * may have buffered data around, queued to be printed right now.
+	 * This is intended to improve performance as it takes more effort
+	 * to write a single character to a file than to write a bunch.
+	 */
+	if (result == 0 && (file->iob_Flags & IOBF_BUFFER_MODE) == IOBF_BUFFER_MODE_NONE)
 	{
-		if(__iob_write_buffer_is_valid(file) && __flush_iob_write_buffer(file) < 0)
+		if (__iob_write_buffer_is_valid(file) && __flush_iob_write_buffer(file) < 0)
 		{
 			SHOWMSG("couldn't flush the write buffer");
 			result = EOF;
 		}
 	}

-	if(stream != NULL)
+	if (stream != NULL)
 		funlockfile(stream);

 	RETURN(result);
 	return(result);
 }
+
+/****************************************************************************/
+
+int
+fputs(const char *s, FILE *stream)
+{
+	int result;
+
+	ENTER();
+
+	SHOWSTRING(s);
+	SHOWPOINTER(stream);
+
+	assert( s != NULL && stream != NULL );
+
+	if (__check_abort_enabled)
+		__check_abort();
+
+	#if defined(CHECK_FOR_NULL_POINTERS)
+	{
+		if (s == NULL || stream == NULL)
+		{
+			__set_errno(EFAULT);
+			goto out;
+		}
+	}
+	#endif /* CHECK_FOR_NULL_POINTERS */
+
+	result = __fputs(s, 0, stream);
+
+	RETURN(result);
+	return(result);
+}
--- a/library/stdio_puts.c
+++ b/library/stdio_puts.c
@@ -44,11 +44,7 @@
 int
 puts(const char *s)
 {
-	FILE * stream = stdout;
-	struct iob * file = (struct iob *)stream;
 	int result = EOF;
-	int buffer_mode;
-	int c;

 	ENTER();

@@ -56,11 +52,6 @@ puts(const char *s)

 	assert( s != NULL );

-	if(__check_abort_enabled)
-		__check_abort();
-
-	flockfile(stream);
-
 	#if defined(CHECK_FOR_NULL_POINTERS)
 	{
 		if(s == NULL)
@@ -71,58 +62,13 @@ puts(const char *s)
 	}
 	#endif /* CHECK_FOR_NULL_POINTERS */

-	assert( __is_valid_iob(file) );
-	assert( FLAG_IS_SET(file->iob_Flags,IOBF_IN_USE) );
-	assert( file->iob_BufferSize > 0 );
-
-	/* If buffering is disabled for an interactive stream
-	   switch to line buffering to improve the readability of
-	   the output: instead of pumping out the entire buffer
-	   break it up into individual lines. */
-	buffer_mode = (file->iob_Flags & IOBF_BUFFER_MODE);
-	if(buffer_mode == IOBF_BUFFER_MODE_NONE)
-	{
-		struct fd * fd = __fd[file->iob_Descriptor];
-
-		__fd_lock(fd);
-
-		if(FLAG_IS_SET(fd->fd_Flags,FDF_IS_INTERACTIVE))
-			buffer_mode = IOBF_BUFFER_MODE_LINE;
-
-		__fd_unlock(fd);
-	}
-
-	if(__fputc_check(stream) < 0)
-		goto out;
-
-	while((c = (*s++)) != '\0')
-	{
-		if(__putc(c,stream,buffer_mode) == EOF)
-			goto out;
-	}
-
-	if(__putc('\n',stream,buffer_mode) == EOF)
+	if (__fputs(s, '\n', stdout) == EOF)
 		goto out;

 	result = OK;

 out:

-	/* Note: if buffering is disabled for this stream, then we still
-	   may have buffered data around, queued to be printed right now.
-	   This is intended to improve performance as it takes more effort
-	   to write a single character to a file than to write a bunch. */
-	if(result == 0 && (file->iob_Flags & IOBF_BUFFER_MODE) == IOBF_BUFFER_MODE_NONE)
-	{
-		if(__iob_write_buffer_is_valid(file) && __flush_iob_write_buffer(file) < 0)
-		{
-			SHOWMSG("couldn't flush the write buffer");
-			result = EOF;
-		}
-	}
-
-	funlockfile(stream);
-
 	RETURN(result);
 	return(result);
 }