On Wed, Apr 11, 2012 at 11:00:42AM +0200, Jan Cholasta wrote:
On 10.4.2012 11:36, Jakub Hrozek wrote:
>Andreas, can you check the src/util/util.c hunk in the first patch? I
>think the atomic io functions were taken from libssh that you maintain..
>
>Honza, can you check out if I didn't miss anything in sss_ssh_knownproxy
>and if it can in fact use the atomic read?
See below.
>
>[PATCH 1/3] sss_atomic_io: Do not fail reads with EPIPE if there is not
>enough data to read
>The hunk in src/util/util.c would apply when the read buffer is bigger
>than the contents of the file we read from. The rest is just unit tests.
Thinking about this, is setting EPIPE when write returns 0
necessary? You can check if sss_atomic_write was sucessful by
checking its return value (sss_atomic_write(fd, buf, n) == n). Maybe
we should not set errno there at all?
I think it makes sense for consistency, so that in any error condition
errno is set. I was rather thinking of returning errno (like adding a
errno_t *err parameter) instead of setting errno.
>
>[PATCH 2/3] Move atomic io function to a separate module
>We'll be using it on various places of the SSSD. The function is in its
>own file to allow using just the one piece without having to drag in the
>whole util.c module.
>There is no functional change in this patch
>
>[PATCH 3/3] Convert read and write operations to sss_atomic_read
>https://fedorahosted.org/sssd/ticket/1209
>
>There are two exceptions that were not converted - one is the read in
>sss_ssh_knownproxy because sss_ssh_knownproxy uses its own poll logic
>that seemed to interfere with what atomic_read_does. The other are read
>and write loops in the sss_client. Those use a logic as to how many bytes
>are left to read and also handle polling themselves.
ACK for sss_ssh_knownhostsproxy. I did not use atomic read in there
for purpose, as it is a blocking call and using it would increase
latency.
Right, thank you.