• Help with review for libcdio transition

    From Gabriel F. T. Gomes@21:1/5 to All on Mon Feb 10 02:30:01 2025
    Hi, Perl Team,

    I'm working on a transition for libcdio, which has a SONAME bump for libiso9660, which libdevice-cdio-perl depends on. I have already made a
    test build of the affected package, and it works. Anyhow, I'd like to
    ask for your help with a change to a data type in libiso9660.

    The data type iso9660_stat_t, which is a struct, has a new
    member/field, thus its size changed.

    I tried reviewing libdevice-cdio-perl's use of this type and it looks
    like they are properly wrapped around SWIG things; however, I'm not very familiar with SWIG, so I could be missing something, even obvious
    things.

    For example, XS(_wrap_ifs_stat_translate) has the following code:

    result = (IsoStat_t *)iso9660_ifs_stat_translate(arg1,(char const (*))arg2);
    {
    // result is of type IsoStatList_t
    iso9660_stat_t *p_statbuf = result;

    if (!result) goto out;

    PPCODE:
    /* Have Perl compute the length of the string using strlen() */
    XPUSHs(sv_2mortal(newSVpv(p_statbuf->filename, 0)));
    XPUSHs(sv_2mortal(newSViv(p_statbuf->lsn)));
    XPUSHs(sv_2mortal(newSViv(p_statbuf->size)));
    XPUSHs(sv_2mortal(newSViv(p_statbuf->secsize)));
    XPUSHs(sv_2mortal(newSViv(p_statbuf->type)));
    XPUSHs(sv_2mortal(newSViv(p_statbuf->tm.tm_sec)));
    XPUSHs(sv_2mortal(newSViv(p_statbuf->tm.tm_min)));
    XPUSHs(sv_2mortal(newSViv(p_statbuf->tm.tm_hour)));
    XPUSHs(sv_2mortal(newSViv(p_statbuf->tm.tm_mday)));
    XPUSHs(sv_2mortal(newSViv(p_statbuf->tm.tm_mon)));
    XPUSHs(sv_2mortal(newSViv(p_statbuf->tm.tm_year)));
    XPUSHs(sv_2mortal(newSViv(p_statbuf->tm.tm_wday)));
    XPUSHs(sv_2mortal(newSViv(p_statbuf->tm.tm_yday)));
    XPUSHs(sv_2mortal(newSViv(p_statbuf->tm.tm_isdst)));
    argvi += 16;
    free (p_statbuf);
    out: ;
    }

    Notice how the several of iso9660_stat_t's fields are accessed, but the
    new field (total_size) is not one of them. There are other absent
    fields in this snippet, so I believe this code is OK, but could you
    have a look, as well.

    I asked for help at the team's IRC channel, and olly explained to me
    that SWIG just generates XS code and that if something could go wrong
    with this ABI change, it would be in one of the XS blocks, such as the
    one I pasted above.

    It would be very nice if some of you could have a look before I start
    the transition.

    Thanks!
    Gabriel

    --- SoupGate-Win32 v1.05
    * Origin: fsxNet Usenet Gateway (21:1/5)
  • From Dominique Dumont@21:1/5 to All on Mon Feb 10 12:30:01 2025
    Hi

    On Monday 10 February 2025 02:01:15 CET Gabriel F. T. Gomes wrote:
    The data type iso9660_stat_t, which is a struct, has a new
    member/field, thus its size changed.

    You mentioned that the new field is "total_size".

    But, reading /usr/include/cdio/iso9660.h from libiso9660-dev:amd64 2.2.0-1~exp1, I found
    this field in struct iso9660_stat_s (ending with "s" not "t") and not in iso9660_stat_t.

    And it looks like libdevice-cdio-perl is not using iso9660_stat_s.

    Which may explain why total_size cannot be found in the C files generated by swig.

    Did I miss something ?

    HTH

    --- SoupGate-Win32 v1.05
    * Origin: fsxNet Usenet Gateway (21:1/5)
  • From Gabriel F. T. Gomes@21:1/5 to All on Mon Feb 10 17:50:01 2025
    But, reading /usr/include/cdio/iso9660.h from libiso9660-dev:amd64 2.2.0-1~exp1, I found this field in struct iso9660_stat_s (ending
    with "s" not "t") and not in iso9660_stat_t.

    True, but iso9660_stat_t is a typedef for the iso9660_stat_s struct:

    https://sources.debian.org/src/libcdio/2.2.0-1~exp1/include/cdio/iso9660.h/#L232

    Which may explain why total_size cannot be found in the C files
    generated by swig.

    Thanks for poiting this out. I had not realized that C files were
    generated. It might be easier for me to review those, than the XS thing.

    --- SoupGate-Win32 v1.05
    * Origin: fsxNet Usenet Gateway (21:1/5)