Permalink
Browse files

Rework fdtrans somewhat for clarity (issue 109)

  • Loading branch information...
1 parent dbddf86 commit fefb2eb7b3a1a951355b86c0023a851d8f59a390 @garlick garlick committed May 15, 2012
Showing with 49 additions and 35 deletions.
  1. +46 −32 libnpfs/fdtrans.c
  2. +2 −2 libnpfs/np.c
  3. +1 −1 libnpfs/npfs.h
View
@@ -41,7 +41,7 @@ struct Fdtrans {
int fdin;
int fdout;
Npfcall *fc;
- int fc_msize;
+ int fc_len; /* used bytes in fc */
};
static int np_fdtrans_recv(Npfcall **fcp, u32 msize, void *a);
@@ -63,7 +63,7 @@ np_fdtrans_create(int fdin, int fdout)
fdt->fdin = fdin;
fdt->fdout = fdout;
fdt->fc = NULL;
- fdt->fc_msize = 0;
+ fdt->fc_len = 0;
npt = np_trans_create(fdt, np_fdtrans_recv,
np_fdtrans_send,
np_fdtrans_destroy);
@@ -92,61 +92,74 @@ np_fdtrans_destroy(void *a)
free(fdt);
}
+/* This function must perform request framing, and return with one request
+ * or an EOF/error. If we read some extra bytes after a full request,
+ * store the extra in fdt->fc and start reading into that next time instead
+ * of allocating a fresh buffer.
+ * N.B. msize starts out at max for the server and can shrink if client
+ * negotiates a smaller one with Tversion. It cannot grow, therefore
+ * the allocated size of cached 'fc' from a preveious call will always be
+ * >= the msize of the current call. See fcall.c::np_version().
+ */
static int
np_fdtrans_recv(Npfcall **fcp, u32 msize, void *a)
{
Fdtrans *fdt = (Fdtrans *)a;
- Npfcall *fc = NULL;
- int n, size, len;
+ Npfcall *fc;
+ u32 size;
+ int n, len;
if (fdt->fc) {
- NP_ASSERT (fdt->fc_msize >= msize);
fc = fdt->fc;
- len = fdt->fc->size;
fdt->fc = NULL;
- size = np_peek_size (fc->pkt, len);
+ len = fdt->fc_len;
+ size = np_peek_size(fc->pkt, len); /* 0 if len < 4 */
+ if (size > msize) {
+ np_uerror(EPROTO);
+ goto error;
+ }
} else {
- if (!(fc = np_alloc_fcall (msize))) {
+ if (!(fc = np_alloc_fcall(msize))) {
np_uerror (ENOMEM);
goto error;
}
len = 0;
size = 0;
}
- while (len < size || len < 4) {
+ while (size == 0 || len < size) {
n = read(fdt->fdin, fc->pkt + len, msize - len);
if (n < 0 && errno == EINTR)
continue;
if (n < 0) {
- np_uerror (errno);
+ np_uerror(errno);
goto error;
}
- if (n == 0) { /* EOF */
- free (fc);
- fc = NULL;
- goto done;
- }
+ if (n == 0)
+ goto eof;
len += n;
- if (size == 0)
+ if (size == 0) {
size = np_peek_size(fc->pkt, len);
- if (size > msize) {
- np_uerror(EPROTO);
- goto error;
+ if (size > msize) {
+ np_uerror(EPROTO);
+ goto error;
+ }
}
}
if (len > size) {
if (!(fdt->fc = np_alloc_fcall (msize))) {
np_uerror(ENOMEM);
goto error;
}
- fdt->fc_msize = msize;
- memcpy (fdt->fc->pkt, fc->pkt + size, len - size);
- fdt->fc->size = len - size;
+ fdt->fc_len = len - size;
+ memcpy (fdt->fc->pkt, fc->pkt + size, fdt->fc_len);
}
fc->size = size;
-done:
*fcp = fc;
return 0;
+eof:
+ free(fc);
+ *fcp = NULL;
+ return 0;
error:
if (fc)
free (fc);
@@ -157,21 +170,22 @@ static int
np_fdtrans_send(Npfcall *fc, void *a)
{
Fdtrans *fdt = (Fdtrans *)a;
- int n, len = 0, size = fc->size;
+ u8 *data = fc->pkt;
+ u32 size = fc->size;
+ int len = 0;
+ int n;
- /* N.B. Caching fc->size avoids a race with mtfsys.c where fc
- * is replaced under us before the do conditional - see issue 72.
- */
- do {
- n = write(fdt->fdout, fc->pkt + len, size - len);
+ while (len < size) {
+ n = write(fdt->fdout, data + len, size - len);
if (n < 0 && errno == EINTR)
continue;
if (n < 0) {
np_uerror(errno);
- return -1;
+ goto error;
}
len += n;
- } while (len < size);
-
+ }
return len;
+error:
+ return -1;
}
View
@@ -1351,10 +1351,10 @@ np_create_runlinkat(void)
return np_post_check(fc, bufp);
}
-int
+u32
np_peek_size(u8 *buf, int len)
{
- int size = 0;
+ u32 size = 0;
if (len >= 4)
size = buf[0] | (buf[1]<<8) | (buf[2]<<16) | (buf[3]<<24);
View
@@ -401,7 +401,7 @@ int np_encode_tpools_str (char **s, int *len, Npstats *stats);
int np_decode_tpools_str (char *s, Npstats *stats);
/* np.c */
-int np_peek_size(u8 *buf, int len);
+u32 np_peek_size(u8 *buf, int len);
Npfcall *np_alloc_fcall(int msize);
int np_deserialize(Npfcall*);
int np_serialize_p9dirent(Npqid *qid, u64 offset, u8 type, char *name, u8 *buf,

0 comments on commit fefb2eb

Please sign in to comment.