On Thu, Sep 29, 2022 at 09:28:24AM +1000, Donald Douwsma wrote:
>
>
> 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.
I don't know. On the one hand I could see an argument for at least
being able to restore multiple streams, but then the dump program has
been screwing that up for years and nobody noticed. I can't tell from
the source code what shape the multistream support is in, so I guess you
have some research to do? <shrug> I suppose you could see what happens
if you try to set up multiple streams, but I have no idea ... what that
means.
Sorry that's a nonanswer. :(
--D
>
> >
> > >
> > > --D
> > >
> > > > + }
> > > > /* all the media files */
> > > > s->mfiles = (invt_mediafile_t *)p;
> > > > --
> > > > 2.31.1
> > > >
> > >
>
