#10 Declined
Repository
vapier/gd gd
Branch
master
Repository
libgd/gd-libgd gd-libgd
Branch
master

fix libgd SONAME

Author
  1. Mike Frysinger avatarMike Frysinger
Reviewers
Description

the 2.1.0 release changed SONAMEs on us. looks like it was an accident.

Comments (5)

  1. Mike Frysinger author

    if the symbols weren't part of the public API in the first place, this doesn't mean the SONAME needs to be bumped. i.e. if people are doing things like "extern safeboolean empty_output_buffer(j_compress_ptr cinfo);" and then calling that func, that is their problem, not ours.

    here's the funcs that have been deleted and are not listed in the headers (and thus do not require the SONAME to be updated):

    any2eucjp
    createwbmp
    empty_output_buffer
    fill_input_buffer
    freewbmp
    gdCalloc
    gdCosT
    gdFontCacheMutex
    gdFontGiantData
    gdFontGiantRep
    gdFontLargeData
    gdFontLargeRep
    gdFontMediumBoldData
    gdFontMediumBoldRep
    gdFontSmallData
    gdFontSmallRep
    gdFontTinyData
    gdFontTinyRep
    gd_getin
    gdImageSubSharpen
    gdMalloc
    gd_putout
    gdRealloc
    gdSinT
    gd_strtok_r
    getmbi
    init_destination
    init_source
    jpeg_gdIOCtx_dest
    jpeg_gdIOCtx_src
    lsqrt
    overflow2
    printwbmp
    putmbi
    readwbmp
    skipheader
    skip_input_data
    term_destination
    term_source
    writewbmp
    

    here's the list of funcs that have been deleted between .2 and .3 which would break ABI:

    gdCacheCreate
    gdCacheDelete
    gdCacheGet
    gdGetBuf
    gdGetByte
    gdGetC
    gdGetInt
    gdGetWord
    gdPutBuf
    gdPutC
    gdPutInt
    gdPutWord
    gdSeek
    gdTell
    Putchar
    Putword
    

    these are odd though in that we still install a header for these symbols. so maybe the source is broken ?

    $ readelf -sW ./src/.libs/gd_io.o
    
    Symbol table '.symtab' contains 29 entries:
       Num:    Value          Size Type    Bind   Vis      Ndx Name
         0: 0000000000000000     0 NOTYPE  LOCAL  DEFAULT  UND 
         1: 0000000000000000     0 FILE    LOCAL  DEFAULT  ABS gd_io.c
         2: 0000000000000000     0 SECTION LOCAL  DEFAULT    1 
         3: 0000000000000000     0 SECTION LOCAL  DEFAULT    2 
         4: 0000000000000000     0 SECTION LOCAL  DEFAULT    3 
         5: 0000000000000000     0 SECTION LOCAL  DEFAULT    4 
         6: 0000000000000000     0 SECTION LOCAL  DEFAULT    6 
         7: 0000000000000000     0 SECTION LOCAL  DEFAULT    7 
         8: 0000000000000000     0 SECTION LOCAL  DEFAULT    8 
         9: 0000000000000000     0 SECTION LOCAL  DEFAULT   10 
        10: 0000000000000000     0 SECTION LOCAL  DEFAULT   12 
        11: 0000000000000000     0 SECTION LOCAL  DEFAULT   14 
        12: 0000000000000000     0 SECTION LOCAL  DEFAULT   15 
        13: 0000000000000000     0 SECTION LOCAL  DEFAULT   13 
        14: 0000000000000000    48 FUNC    GLOBAL HIDDEN     1 Putword
        15: 0000000000000040    16 FUNC    GLOBAL HIDDEN     1 Putchar
        16: 0000000000000060    16 FUNC    GLOBAL HIDDEN     1 gdPutC
        17: 0000000000000080    42 FUNC    GLOBAL HIDDEN     1 gdPutWord
        18: 00000000000000c0    68 FUNC    GLOBAL HIDDEN     1 gdPutInt
        19: 0000000000000120     5 FUNC    GLOBAL HIDDEN     1 gdGetC
        20: 0000000000000140    28 FUNC    GLOBAL HIDDEN     1 gdGetByte
        21: 0000000000000160    65 FUNC    GLOBAL HIDDEN     1 gdGetWord
        22: 00000000000001c0    79 FUNC    GLOBAL HIDDEN     1 gdGetWordLSB
        23: 0000000000000220    97 FUNC    GLOBAL HIDDEN     1 gdGetInt
        24: 00000000000002a0   143 FUNC    GLOBAL HIDDEN     1 gdGetIntLSB
        25: 0000000000000340    17 FUNC    GLOBAL HIDDEN     1 gdPutBuf
        26: 0000000000000360    17 FUNC    GLOBAL HIDDEN     1 gdGetBuf
        27: 0000000000000380     6 FUNC    GLOBAL HIDDEN     1 gdSeek
        28: 00000000000003a0     6 FUNC    GLOBAL HIDDEN     1 gdTell
    
  2. Ondřej Surý

    is their problem, not ours.

    That's irresponsible and the problems which might arise from unexpected ABI changes are much bigger than bumping SONAME.

    these are odd though in that we still install a header for these symbols. so maybe the source is broken ?

    Pierre Joye Do we have to install gdcache.h and gd_io.h?

  3. Mike Frysinger author

    that isn't irresponsible at all. the ABI isn't defined by accidental symbol exports, it's defined by the API we say it is. people using obviously internal symbols are violating the API which means they deserve to break. otherwise, you're basically saying that if we have a compiler that lacks symbol visibility (which configure checks for) then we need to bump the SONAME pretty much every release because a new internal func is added/removed/changed. that is an unreasonable position that no other project takes, and bumping the SONAME like this is irresponsible on our part. library authors have a duty to keep the SONAME stable whenever possible.

    in sampling the projects that i happen to have installed on my system that use GD, i don't see any of them using gdcache.h or gd_io.h, or the funcs defined therein.

    autotrace-0.31.1
    dvipng-1.14
    EMBOSS-6.3.1
    freetype-2.4.11
    GD-2.46
    glibc-2.17
    gnuplot-4.6.1
    graphviz-2.28.0
    icinga-1.8.4
    jffnms-0.9.3
    libgphoto2-2.5.1.1
    libwmf-0.2.8.4
    mrtg-2.17.4
    nagios-3.3.1
    ntop-4.0.3
    php-5.3.26
    php-5.4.16
    php-5.5.0
    pstoedit-3.61
    sarg-2.3.4
    vnstat-1.11
    zabbix-2.0.6
    

    php-5.4.x and older do search for gdCacheCreate (that configure logic is weird) and uses it to control whether to build its local copy of caching code. not sure whether this will break the build, but they've dropped the logic with php-5.5.x.

    note that the struct gdIOCtx in gd_io.h is still needed. that's how people use the funcs that end in "Ctx".

    googling for the headers and some of the funcs therein only shows gd itself. so i guess we can try deleting the headers and see if anyone complains.

  4. Pierre Joye

    Hi!

    Sorry for the late reply here.

    Mike, you have a very good point here. gdIOCtx is required. However we do not expose the other functions declared there, not necessary. What's about moving this struct declaration to gd.h and make it public?

    About relying on compiler features to make something public or not, that's partially the case. Basically the main idea is to have headers with only what could be used. The same should be about the members of a struct (APIs should be used to access them) but that's not possible to restrict that, so we should clearly document the how and why.

  5. Mike Frysinger author

    moving the gdIOCtx to gd.h seems like the easiest route to dropping the gd_io.h header

    for structs, you can make them entirely opaque, or just certain fields. this can be accomplished a few different ways, but the easiest is probably via just some ifdefs. you can even remove the struct entirely from visibility by replacing it with alloc/free/get/set helpers. not saying we need to go that far, just that it's possible. as you say, what exactly we expect to be part of the API in the struct should be documented. certainly the size/layout of the struct is part of the ABI today.

    on a related note, i see that we're using HAVE_xxx defines in gd.h. that doesn't work. we should do:

    -# ifdef HAVE_VISIBILITY
    +# if @USE_VISIBILITY@
    

    and unconditionally define USE_VISIBILITY to 0 or 1.

Tip: Filter by directory path e.g. /media app.js to search for public/media/app.js.
Tip: Use camelCasing e.g. ProjME to search for ProjectModifiedEvent.java.
Tip: Filter by extension type e.g. /repo .js to search for all .js files in the /repo directory.
Tip: Separate your search with spaces e.g. /ssh pom.xml to search for src/ssh/pom.xml.
Tip: Use ↑ and ↓ arrow keys to navigate and return to view the file.
Tip: You can also navigate files with Ctrl+j (next) and Ctrl+k (previous) and view the file with Ctrl+o.
Tip: You can also navigate files with Alt+j (next) and Alt+k (previous) and view the file with Alt+o.