comments on proposed uthread_write.c changes

[Dan Langille]

On 7 Sep 2003 at 12:32, Daniel Eischen wrote:

> On Sun, 7 Sep 2003, Dan Langille wrote:
> 
> > A problem with pthreads and EOT has been identified.  See PR 56274.  It
> > was suggested the solution was probably just a matter of changing one of
> > the >0 tests to >=0 in uthread_write.c
> > 
> > Any comments on that?
> 
> I don't know that a return of 0 isn't valid for other devices.
> If this is the case, a return of 0 for blocking writes may break
> other applications.
> 
> The patch isn't quite correct (at least looking at -current srcs).
> Lines 98-99 are:
> 
> 			if (blocking && ((n < 0 && (errno == EWOULDBLOCK ||
> 			    errno == EAGAIN)) || (n >= 0 && num < nbytes))) {
> 
> This will get entered first if n == 0, and I don't think your
> proposed patch would have any effect.  I think you would have
> to change the "n >= 0" above to be "n > 0" in conjunction with
> your patch.

Ahh thank you.  That explains why the test results with the original 
patch did not differ from -STABLE or 5.1-RELEASE.  After adding your 
suggestions, we have had success.

The points to note:

1. The status that stopped the writing was 0
2. It wrote 17,256 blocks, and read 17,256 blocks.

Point 1 is key to determining EOT.  Point 2 is what you always want 
to have...

[dan at undef:~/tape-test] $ sudo ./tapetest /dev/nsa0
*rewind
Rewound /dev/nsa0
*rawfill
Begin writing blocks of 64512 bytes.
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
+++++
weof_dev
Wrote EOF to /dev/nsa0
Write failed.  Last block written=17256. stat=0 ERR=Unknown error: 0
*rewind
Rewound /dev/nsa0
*scan
Starting scan at file 0
17256 blocks of 64512 bytes in file 0
End of File mark.
End of File mark.
End of tape
Total files=1, blocks=17256, bytes = 1113219072
*

> This could be solved at the application level by selecting on
> the tape device and performing non-blocking writes to it.  Since
> the application knows that a return of 0 is end-of-tape, it
> must also know the difference between talking to a tape device
> and talking to a regular file, socket, etc.

True.  But *if* the code is wrong, it should be fixed.

FWIW, the patch follows.  As always, opinions and suggestions are 
welcome.

--- uthread_write.c.org	Sun Sep  7 10:58:31 2003
+++ uthread_write.c	Sun Sep  7 15:41:34 2003
@@ -93,7 +93,7 @@
 			 * write:
 			 */
 			if (blocking && ((n < 0 && (errno == EWOULDBLOCK ||
-			    errno == EAGAIN)) || (n >= 0 && num < nbytes))) {
+			    errno == EAGAIN)) || (n > 0 && num < nbytes))) {
 				curthread->data.fd.fd = fd;
 				_thread_kern_set_timeout(NULL);
 
@@ -131,7 +131,7 @@
 			 * If there was an error, return partial success
 			 * (if any bytes were written) or else the error:
 			 */
-			} else if (n < 0) {
+			} else if (n <= 0) {
 				if (num > 0)
 					ret = num;
 				else

-- 
Dan Langille : http://www.langille.org/