On 29/09/2022 09:12, Donald Douwsma wrote:
On 29/09/2022 01:23, Darrick J. Wong wrote:
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@xxxxxxxxxx>
---
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 ^
nod
+ (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], ...); ?
Thanks! Yes, that's wrong, like the existing code it only worked/works
because there's only ever one stream. From the manpage "The third level
is media stream (currently only one stream is supported)". Will fix.
Or should I just drop this clean-up? I think what I'm saying is right,
but its a clean-up for a feature that cant be used. I doubt anyone is
going to add multiple stream support now, whatever that was intended
for.
--D
+ }
/* all the media files */
s->mfiles = (invt_mediafile_t *)p;
--
2.31.1
