Re: svn commit: r368410 - head/stand/libsa/zfs

[Jessica Clarke]

On 7 Dec 2020, at 11:25, Toomas Soome  wrote:
> @@ -183,28 +215,29 @@ xdr_u_int(xdr_t *xdr, unsigned *ip)
> static bool
> xdr_int64(xdr_t *xdr, int64_t *lp)
> {
> - int hi;
> - unsigned lo;
>   bool rv = false;
> 
> - if (xdr->xdr_idx + sizeof (int64_t) > xdr->xdr_buf + xdr->xdr_buf_size)
> + if (xdr->xdr_idx + sizeof(int64_t) > xdr->xdr_buf + xdr->xdr_buf_size)
>   return (rv);
> 
>   switch (xdr->xdr_op) {
>   case XDR_OP_ENCODE:
>   /* Encode value *lp, store to buf */
> - hi = *lp >> 32;
> - lo = *lp & UINT32_MAX;
> - xdr->xdr_idx += xdr->xdr_putint(xdr, hi);
> - xdr->xdr_idx += xdr->xdr_putint(xdr, lo);
> + if (xdr->xdr_putint == _putint)
> + *(int64_t *)xdr->xdr_idx = htobe64(*lp);
> + else
> + *(int64_t *)xdr->xdr_idx = *lp;

I don't know the details here, but inspecting the callback function and
comparing it against a known one to decide what to do is generally not
good practice. Can this be pushed down into the function in question,
up into the caller or additional information passed to be explicit
about this behaviour rather than brittle magic behaviour?

Jess




Jess

___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

svn commit: r368410 - head/stand/libsa/zfs

[Toomas Soome]

Author: tsoome
Date: Mon Dec  7 11:25:18 2020
New Revision: 368410
URL: https://svnweb.freebsd.org/changeset/base/368410

Log:
  loader: xdr_array is missing count
  
  The integer arrays are encoded in nvlist as counted array ,
  loader xdr_array() is missing the count. This will affect the pool import when
  there are hole devices in pool.
  
  Also fix the new data add and print functions.

Modified:
  head/stand/libsa/zfs/nvlist.c

Modified: head/stand/libsa/zfs/nvlist.c
==
--- head/stand/libsa/zfs/nvlist.c   Mon Dec  7 11:18:51 2020
(r368409)
+++ head/stand/libsa/zfs/nvlist.c   Mon Dec  7 11:25:18 2020
(r368410)
@@ -63,7 +63,7 @@ static int
 _getint(struct xdr *xdr, int *ip)
 {
*ip = be32dec(xdr->xdr_idx);
-   return (sizeof (int));
+   return (sizeof(int));
 }
 
 static int
@@ -72,14 +72,14 @@ _putint(struct xdr *xdr, int i)
int *ip = (int *)xdr->xdr_idx;
 
*ip = htobe32(i);
-   return (sizeof (int));
+   return (sizeof(int));
 }
 
 static int
 _getuint(struct xdr *xdr, unsigned *ip)
 {
*ip = be32dec(xdr->xdr_idx);
-   return (sizeof (unsigned));
+   return (sizeof(unsigned));
 }
 
 static int
@@ -88,9 +88,41 @@ _putuint(struct xdr *xdr, unsigned i)
unsigned *up = (unsigned *)xdr->xdr_idx;
 
*up = htobe32(i);
-   return (sizeof (int));
+   return (sizeof(int));
 }
 
+static int
+_getint_mem(struct xdr *xdr, int *ip)
+{
+   *ip = *(int *)xdr->xdr_idx;
+   return (sizeof(int));
+}
+
+static int
+_putint_mem(struct xdr *xdr, int i)
+{
+   int *ip = (int *)xdr->xdr_idx;
+
+   *ip = i;
+   return (sizeof(int));
+}
+
+static int
+_getuint_mem(struct xdr *xdr, unsigned *ip)
+{
+   *ip = *(unsigned *)xdr->xdr_idx;
+   return (sizeof(unsigned));
+}
+
+static int
+_putuint_mem(struct xdr *xdr, unsigned i)
+{
+   unsigned *up = (unsigned *)xdr->xdr_idx;
+
+   *up = i;
+   return (sizeof(int));
+}
+
 /*
  * XDR data translations.
  */
@@ -131,7 +163,7 @@ xdr_int(xdr_t *xdr, int *ip)
bool rv = false;
int *i = (int *)xdr->xdr_idx;
 
-   if (xdr->xdr_idx + sizeof (int) > xdr->xdr_buf + xdr->xdr_buf_size)
+   if (xdr->xdr_idx + sizeof(int) > xdr->xdr_buf + xdr->xdr_buf_size)
return (rv);
 
switch (xdr->xdr_op) {
@@ -160,7 +192,7 @@ xdr_u_int(xdr_t *xdr, unsigned *ip)
bool rv = false;
unsigned *u = (unsigned *)xdr->xdr_idx;
 
-   if (xdr->xdr_idx + sizeof (unsigned) > xdr->xdr_buf + xdr->xdr_buf_size)
+   if (xdr->xdr_idx + sizeof(unsigned) > xdr->xdr_buf + xdr->xdr_buf_size)
return (rv);
 
switch (xdr->xdr_op) {
@@ -183,28 +215,29 @@ xdr_u_int(xdr_t *xdr, unsigned *ip)
 static bool
 xdr_int64(xdr_t *xdr, int64_t *lp)
 {
-   int hi;
-   unsigned lo;
bool rv = false;
 
-   if (xdr->xdr_idx + sizeof (int64_t) > xdr->xdr_buf + xdr->xdr_buf_size)
+   if (xdr->xdr_idx + sizeof(int64_t) > xdr->xdr_buf + xdr->xdr_buf_size)
return (rv);
 
switch (xdr->xdr_op) {
case XDR_OP_ENCODE:
/* Encode value *lp, store to buf */
-   hi = *lp >> 32;
-   lo = *lp & UINT32_MAX;
-   xdr->xdr_idx += xdr->xdr_putint(xdr, hi);
-   xdr->xdr_idx += xdr->xdr_putint(xdr, lo);
+   if (xdr->xdr_putint == _putint)
+   *(int64_t *)xdr->xdr_idx = htobe64(*lp);
+   else
+   *(int64_t *)xdr->xdr_idx = *lp;
+   xdr->xdr_idx += sizeof(int64_t);
rv = true;
break;
 
case XDR_OP_DECODE:
/* Decode buf, return value to *ip */
-   xdr->xdr_idx += xdr->xdr_getint(xdr, );
-   xdr->xdr_idx += xdr->xdr_getuint(xdr, );
-   *lp = (((int64_t)hi) << 32) | lo;
+   if (xdr->xdr_getint == _getint)
+   *lp = be64toh(*(int64_t *)xdr->xdr_idx);
+   else
+   *lp = *(int64_t *)xdr->xdr_idx;
+   xdr->xdr_idx += sizeof(int64_t);
rv = true;
}
return (rv);
@@ -213,27 +246,29 @@ xdr_int64(xdr_t *xdr, int64_t *lp)
 static bool
 xdr_uint64(xdr_t *xdr, uint64_t *lp)
 {
-   unsigned hi, lo;
bool rv = false;
 
-   if (xdr->xdr_idx + sizeof (uint64_t) > xdr->xdr_buf + xdr->xdr_buf_size)
+   if (xdr->xdr_idx + sizeof(uint64_t) > xdr->xdr_buf + xdr->xdr_buf_size)
return (rv);
 
switch (xdr->xdr_op) {
case XDR_OP_ENCODE:
/* Encode value *ip, store to buf */
-   hi = *lp >> 32;
-   lo = *lp & UINT32_MAX;
-   xdr->xdr_idx += xdr->xdr_putint(xdr, hi);
-   xdr->xdr_idx += xdr->xdr_putint(xdr, lo);
+   if (xdr->xdr_putint == _putint)
+   *(uint64_t