[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Help with review for libcdio transition



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


Reply to: