Re: [PATCH 2/3] xfsrestore: stobj_unpack_sessinfo cleanup

["Darrick J. Wong" <djwong@kernel.org>]

On Wed, Sep 28, 2022 at 03:53:06PM +1000, Donald Douwsma wrote:
> stobj_unpack_sessinfo should be the reverse of stobj_pack_sessinfo, make
> this clearer with respect to the session header and streams processing.
> 
> signed-off-by: Donald Douwsma <ddouwsma@redhat.com>
> ---
>  inventory/inv_stobj.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/inventory/inv_stobj.c b/inventory/inv_stobj.c
> index 5075ee4..521acff 100644
> --- a/inventory/inv_stobj.c
> +++ b/inventory/inv_stobj.c
> @@ -1065,25 +1065,26 @@ stobj_unpack_sessinfo(
>  		return BOOL_FALSE;
>  	}
>  
> +	/* get the seshdr and then, the remainder of the session */
>  	xlate_invt_seshdr((invt_seshdr_t *)p, (invt_seshdr_t *)tmpbuf, 1);
>  	bcopy(tmpbuf, p, sizeof(invt_seshdr_t));
> -
> -	/* get the seshdr and then, the remainder of the session */
>  	s->seshdr = (invt_seshdr_t *)p;
>  	s->seshdr->sh_sess_off = -1;
>  	p += sizeof(invt_seshdr_t);
>  
> -
>  	xlate_invt_session((invt_session_t *)p, (invt_session_t *)tmpbuf, 1);
>  	bcopy (tmpbuf, p, sizeof(invt_session_t));
>  	s->ses = (invt_session_t *)p;
>  	p += sizeof(invt_session_t);
>  
>  	/* the array of all the streams belonging to this session */
> -	xlate_invt_stream((invt_stream_t *)p, (invt_stream_t *)tmpbuf, 1);
> -	bcopy(tmpbuf, p, sizeof(invt_stream_t));
>  	s->strms = (invt_stream_t *)p;
> -	p += s->ses->s_cur_nstreams * sizeof(invt_stream_t);
> +	for (i = 0; i < s->ses->s_cur_nstreams; i++) {
> +		xlate_invt_stream((invt_stream_t *)p, 

Nit: trailing whitespace                        here ^

> +				  (invt_stream_t *)tmpbuf, 1);
> +		bcopy(tmpbuf, p, sizeof(invt_stream_t));
> +		p += sizeof(invt_stream_t);

Ok, so we translate p into tmpbuf, then bcopy tmpbuf back to p.  That
part makes sense, but I am puzzled by what stobj_pack_sessinfo does:

	for (i = 0; i < ses->s_cur_nstreams; i++) {
		xlate_invt_stream(strms, (invt_stream_t *)sesbuf, 1);
		sesbuf += sizeof(invt_stream_t);
	}

Why isn't that callsite xlate_invt_stream(&strms[i], ...); ?

--D

> +	}
>  
>  	/* all the media files */
>  	s->mfiles = (invt_mediafile_t *)p;
> -- 
> 2.31.1
>