From mboxrd@z Thu Jan 1 00:00:00 1970 From: compudj@krystal.dyndns.org (Mathieu Desnoyers) Date: Tue, 7 Sep 2010 11:43:22 -0400 Subject: [ltt-dev] [UST PATCH] remove duplicate return In-Reply-To: <4C854BA0.4040701@polymtl.ca> References: <1283729203-4435-1-git-send-email-douglas.santos@polymtl.ca> <20100906004907.GB14418@Krystal> <1283735019.1893.3.camel@Nokia-N900-42-11> <20100906024254.GA25072@Krystal> <20100906152927.GA22968@Krystal> <4C854BA0.4040701@polymtl.ca> Message-ID: <20100907154322.GB6371@Krystal> * Pierre-Marc Fournier (pierre-marc.fournier at polymtl.ca) wrote: > On 09/06/2010 11:29 AM, Mathieu Desnoyers wrote: >> * Mathieu Desnoyers (compudj at krystal.dyndns.org) wrote: >>> * Pierre-Marc Fournier (pierre-marc.fournier at polymtl.ca) wrote: >>>> I disagree with you Mathieu. These retvals are the same as i/o >>>> syscalls (read/write/send/recv/...)and therefore should in my opinion >>>> remain as is. >>> >>> Well, this function is not technically the same as i/o syscalls at all. >>> It uses I/O syscalls, but it is not an I/O syscall per se, so the return >>> value transformation to a more standard pattern (neg err val, 0 ok) >>> should happen right in this function rather than to let all callers >>> handle this. I/O syscalls use positive return values to indicate the >>> number of bytes read/written/etc. Here, this function arbitrarily choose >>> 1 to indicate that "something has been sent" without caring about the >>> amount of data moved at all. >>> >>> So as it doesn't need the whole positive range to spell out the amount >>> of data moved, it doesn't need to do the same special-cases that the I/O >>> syscalls are doing. It adds a lot of error values management oddness >>> without adding anything. >>> >>> So even though I agree with you that this function is close to the I/O >>> system calls because it calls it, it is very far from the I/O syscalls >>> semantically (we don't care about the number of bytes written), and even >>> though we might be tempted to use the same error values as system calls >>> for them, the fact that we just don't care about the number of bytes >>> written combined with the fact that standardizing error value across the >>> code makes it much easier to follow and to write just call for this >>> change. >> >> By the way, looking at include/share.h:patient_write(), in the case >> where write returns 0, I think we should consider this as a success and >> loop again to retry write rather than consider that an error occurred. >> The same apply to patient_send(). See the manpages for details: >> >> write(2): >> >> RETURN VALUE >> On success, the number of bytes written is returned (zero indicates >> nothing was written). On error, -1 is returned, and errno is set >> appropriately. >> >> If count is zero and fd refers to a regular file, then write() may >> return a failure status if one of the errors below is detected. If no >> errors are detected, 0 will be returned without causing any other >> effect. If count is zero and fd refers to a file other than a regular >> file, the results are not specified. >> >> send(2): >> RETURN VALUE >> On success, these calls return the number of characters sent. On >> error, -1 is returned, and errno is set appropriately. >> > > No. 0 indicates end of stream By end of stream, you mean this sequence ? server: (block SIGPIPE) sockwrite = socket(AF_UNIX, SOCK_STREAM, 0); addr.sun_family = AF_UNIX; strcpy(addr.sun_path, "./mysocket"); /* wait for client to create the unix socket */ sleep(2); connect(sockwrite, (struct sockaddr *)&addr, sizeof(addr));) /* let's do one write which will be consumed by the client */ write(sockwrite, "blah", sizeof("blah")); /* wait for client to close FD */ sleep(2); ret = write(sockwrite, "blah", sizeof("blah")); ( or even with ret = send(sockwrite, "blah", sizeof("blah"), MSG_NOSIGNAL); ) printf("write ret %ld errno %ld\n", ret, errno); client: (block SIGPIPE) sockserv = socket(AF_UNIX, SOCK_STREAM, 0); addr.sun_family = AF_UNIX; strcpy(addr.sun_path, "./mysocket"); bind(sockserv, (struct sockaddr *)&addr, sizeof(addr)); listen(sockserv, 1); sockread = accept(sockserv, NULL, NULL); recv(sockread, buf, sizeof(buf), 0); close(sockread); The server printf returns: "write ret -1 errno 32" So the return value is -1, with an error number: EPIPE: Broken pipe > and looping on it will result in an > infinite loop. You need to refer to the specific backend driver to > understand these specific semantics. The read/write manpages are > notorious for their non-clarity about this. This is what I just did by creating my small test program. Unless I am misunderstanding your definition of "end of stream", your assumptions about the way the unix sockets work seem incorrect. Note that as the manpage says, if the file descriptor refers to a regular file, then write() can return 0, with an error number set, which could indicate that an error occured. If the file descriptor refers to a file other than a regular file, the results are unspecified. So I think the sane way to handle this would be to check if errno is 0 when write returns 0. If errno != 0, then we have an error, but if errno == 0, I'd consider that 0 bytes were written and retry. By the way, you forgot to reply to my first message above which covers the topic of semantics, so I'm re-pasting it here. (about ustcomm_send_request()) Well, this function is not technically the same as i/o syscalls at all. It uses I/O syscalls, but it is not an I/O syscall per se, so the return value transformation to a more standard pattern (neg err val, 0 ok) should happen right in this function rather than to let all callers handle this. I/O syscalls use positive return values to indicate the number of bytes read/written/etc. Here, this function arbitrarily choose 1 to indicate that "something has been sent" without caring about the amount of data moved at all. So as it doesn't need the whole positive range to spell out the amount of data moved, it doesn't need to do the same special-cases that the I/O syscalls are doing. It adds a lot of error values management oddness without adding anything. So even though I agree with you that this function is close to the I/O system calls because it calls it, it is very far from the I/O syscalls semantically (we don't care about the number of bytes written), and even though we might be tempted to use the same error values as system calls for them, the fact that we just don't care about the number of bytes written combined with the fact that standardizing error value across the code makes it much easier to follow and to write just call for this change. Thanks, Mathieu > > pmf > -- Mathieu Desnoyers Operating System Efficiency R&D Consultant EfficiOS Inc. http://www.efficios.com