Page MenuHomeFreeBSD

Add `__' prefix to the ABI of libmd
AbandonedPublic

Authored by ume on Jan 16 2015, 5:32 AM.

Details

Reviewers
kuriyama
hrs
Summary

DESCRIPTION

Add `__' prefix to the ABI of libmd to avoid name conflicts with libcrypto.

( Related to D2216: Avoid symbol clashes between libmd and libcrypto )

Diff Detail

Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

kuriyama updated this revision to Diff 3207.Jan 16 2015, 5:32 AM
kuriyama retitled this revision from to Change OpenSSL derived digest functions to return boolean values.
kuriyama updated this object.
kuriyama edited the test plan for this revision. (Show Details)
kuriyama updated this object.Jan 16 2015, 5:35 AM
kuriyama updated this object.
gleb added a subscriber: gleb.Jan 16 2015, 7:19 PM

I don't think it is a good idea.

There is too much code out there written under assumption that those functions can't fail and original interface emphasizes the intent. *_Init is a little bit trickier because it requires parameter validation, etc. But in our case it still never fails.

ume added a subscriber: ume.Jan 22 2015, 5:53 PM

I think changing API is not good idea. How about the attached patch? It changes function names to avoid conflicts with libcrypto.

gleb added a comment.Jan 31 2015, 8:18 PM

"__" prefix is reserved and should not be used in the library. If you want to rename the symbols, let's simply rename them, as well as all API consumers.

Have you considered adding symbol versioning to libmd? I think it should resolve original install issue -- I assume it to be two shared libraries providing the same symbol.

BTW there should be a way to update patch in review, instead of posting link to a file.

ume added a reviewer: hrs.Feb 3 2015, 4:21 PM
ume added a comment.Feb 3 2015, 4:38 PM

The use of `__' is intentional, here. It seems you are confusing ABI
with API. Though it changes the ABI of libmd, as far as the
applications include the corresponding header file, it doesn't change
the API.
Use of symbol versioning is overkill for this purpose, IMHO.

ume commandeered this revision.Feb 3 2015, 5:13 PM
ume added a reviewer: kuriyama.
ume updated this revision to Diff 3614.Feb 3 2015, 5:23 PM
ume retitled this revision from Change OpenSSL derived digest functions to return boolean values to Add `__' prefix to the API of libmd.
ume updated this object.
ume updated this object.
ume retitled this revision from Add `__' prefix to the API of libmd to Add `__' prefix to the ABI of libmd.Feb 3 2015, 5:25 PM
gleb added a comment.Feb 4 2015, 3:03 AM

You would need to bump shlib major.
"__" is reserved I will be against this patchset as long as it stays there.
What is intent of providing weak_reference?
I don't see how listing those symbols in single file in order to enable symbol versioning can be more complicated than current approach.

ume added a comment.Feb 4 2015, 3:28 PM

No, we don't need to bump shlib major until removing weak_reference.
The weak_reference is for keeping ABI backward compatibility.

kuriyama edited edge metadata.Feb 5 2015, 1:24 AM

FWIW, I tested ume's patch on my environment, and it works fine.

kuriyama added a comment.EditedMar 26 2015, 2:42 PM

Gleb, still you against this patch?

'__' is reserved for implementation (by POSIX.1-2008), so I think this is correct usage. We already use a lot of these prefix symbols in our libraries.

kuriyama updated this object.Apr 16 2015, 2:30 AM
kuriyama edited edge metadata.