[FFmpeg-devel] Fix libssh static linkage on Windows

[Michael Niedermayer]

On Tue, Feb 11, 2014 at 01:35:03AM +1100, Matt Oliver wrote:
> On 11 February 2014 00:39, Michael Niedermayer <michaelni at gmx.at> wrote:
> 
> > On Thu, Feb 06, 2014 at 12:13:35PM +1100, Matt Oliver wrote:
> > > On 6 February 2014 05:53, Reimar Döffinger <Reimar.Doeffinger at gmx.de>
> > wrote:
> > >
> > > > On Thu, Feb 06, 2014 at 01:21:34AM +1100, Matt Oliver wrote:
> > > > > On windows there are 2 ways to link to a function found in an
> > external
> > > > > shared library (.dll on windows). The first is to generate an export
> > > > > library with your shared dll. This export library looks just like a
> > > > static
> > > > > lib but all the functions found in it point out to the external dll.
> > The
> > > > > second method is to declare each function as dllimport. This bypasses
> > > > > needing to link to the export lib by explicitly stating its from a
> > dll in
> > > > > the code.
> > > > >
> > > > > dllimport is placed in front of any function or data declarations in
> > code
> > > > > to specify that they are found in an external .dll (.so shared
> > library on
> > > > > linux). If a function is declared as dllimport then the linker will
> > only
> > > > > look for that function in an external dll even if that function has
> > been
> > > > > provided with a static library. So linking will always fail if using
> > a
> > > > > static library to provide a function as the linker will only allow
> > for an
> > > > > external shared function.
> > > >
> > > > Are you sure about that? Because that is not my experience.
> > > > As long as you use dllexport for the corresponding functions while
> > > > compiling the static lib I believe it should work just fine.
> > > > Note that you like this risk that libavformat ends up exporting
> > > > the libssh functions, so it's not exactly such a great idea,
> > > > but the underlying issue should be using dllimport where you use
> > > > the function but not using dllexport where you defined it.
> > > > Also, to quote Microsoft:
> > > > > dllexport of a function exposes the function with its decorated name.
> > > > For C++ functions, this includes name mangling. For C functions or
> > > > functions that are declared as extern "C", this includes
> > platform-specific
> > > > decoration that's based on the calling convention. If you don't want
> > name
> > > > decoration, use a .def file (EXPORTS keyword).
> > > >
> > > > So it's basically a name mangling issue (for C I believe it's just
> > > > that a _ is prepended).
> > > > It also hints that just using a .def file and none of this
> > > > dllimport/dllexport
> > > > stuff would avoid the issue as well.
> > > > _______________________________________________
> > > > ffmpeg-devel mailing list
> > > > ffmpeg-devel at ffmpeg.org
> > > > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> > > >
> > >
> > > I probably shouldn't send emails at 2am after a late night as my last one
> > > was a bit off. Your right in that dllimport is just a name mangling thing
> > > that works providing you compiled the original static lib with dllexport.
> > > Having dllimport means that it will only link with static libs compiled
> > > using LIBSSH_EXPORTS (dllexport) as opposed to the more obvious
> > > LIBSSH_STATIC. However building a static with that define requires the
> > > linking program to also use that define. That is the reason for the patch
> > > as with the define you can link against a static lib without exports
> > while
> > > also being able to link against a shared lib that does use exports.
> > >
> > > Given that the shared lib project provided by the libssh project defines
> > > LIBSSH_STATIC then any static lib using the default libssh builds wont be
> > > compatible with ffmpeg due to the conflicting use of dllexport. But by
> > > adding the patch then not only will standard libssh static builds work
> > but
> > > the default shared builds will still work as well.
> >
> > is LIBSSH_STATIC officially defined to be safe to be defined for
> > dynamic linking ? (it does not matter if its now with your installed
> > libssh on your box)
> > if yes please point me to the documentation
> > if its defined not to be safe then this requires some #if id guess
> > if its undefined then i think its best if you contact the libssh
> > developers and ask so it gets clarified (in their docs) if
> > unconditional use of LIBSSH_STATIC is how its supposed to be used.
> >
> > [...]
> >
> > --
> > Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
> >
> > I have often repented speaking, but never of holding my tongue.
> > -- Xenocrates
> >
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel at ffmpeg.org
> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
> >
> is LIBSSH_STATIC officially defined to be safe to be defined for
> > dynamic linking ?
> 
> 
> Officially according to microsoft it is safe as any code in a shared dll on

microsoft doesnt document LIBSSH_STATIC


[...]
> 
> libssh however has no stance on not using dllimport on a shared library and
> there docs only state one or the other:
> http://api.libssh.org/master/libssh_linking.html

you are quite fixed on dllimport, yet your patch does not add dllimport
it adds LIBSSH_STATIC.

You seem to mix documented API with current behavior

My question was, if adding LIBSSH_STATIC unconditional is safe per
official libssh documentation
AFAIK its not. so your patch uses it in a undocumeted way

Using undocumeted things is not good. We would like to have wide
compatibility. Both with future and past versions of libs and with
potential forks and drop in replacements if such ever exist

I suggest again to try to get this clarified at libssh
it sure would help people if their docs spell out if using it for
dynamic libs or non windows is safe, unsafe or undefined

either way above are just my 2 cent

patch review left to maintainer

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

While the State exists there can be no freedom; when there is freedom there
will be no State. -- Vladimir Lenin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20140210/32c161ef/attachment.asc>