Opened 10 years ago

Closed 9 years ago

#190 closed defect (fixed)

libdns is not compatible with libdns

Reported by: jreed Owned by: jreed
Priority: medium Milestone: 05. 3rd Incremental Release: Serious Secondary
Component: libdns++ Version:
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity:
Sub-Project: Feature Depending on Ticket:
Estimated Difficulty: Add Hours to Ticket:
Total Hours: Internal?:

Description

Consider using a different name or installing with a different shared library major number.

Maybe call it libdns++ ?

The example I saw is when I ran my host tool (when my BIND 10 libdns was not available):

/usr/lib/libdns.so.0: Undefined symbol "isc_categories" (symnum = 186)

Subtickets

Change History (7)

comment:1 in reply to: ↑ description ; follow-up: Changed 10 years ago by jinmei

Replying to jreed:

Consider using a different name or installing with a different shared library major number.

Maybe call it libdns++ ?

The example I saw is when I ran my host tool (when my BIND 10 libdns was not available):

/usr/lib/libdns.so.0: Undefined symbol "isc_categories" (symnum = 186)

Yes, I was concerned about the possible conflict with the existing libdns (it's a BIND9 library, right?).

But at the same time, this type of conflict can generally happen, and it doesn't make sense to modify the library name for every such conflict as we find it. Doesn't it make more sense to avoid the conflict at the package name level (such as "bind10") and by specifying specific path to the library including the package name? For example, we can use rpath when it's available.

I think I like the name of libdns++ though:-)

comment:2 in reply to: ↑ 1 Changed 9 years ago by jinmei

Replying to jinmei:

I think I like the name of libdns++ though:-)

branches/trac190 is ready for review. The changeset is r2120. It should be quite trivial (excpet a "bad knowhow" of automake that we need to convert ++ to in XXX_SOURCES).

The proposed changelog entry is as follows:

  55.	[func]*		jinmei
	lib/dns: renamed the library name to libdns++ to avoid confusion
	with the same name of library of BIND 9.
	(Trac #190, svn TBD)

Additional general note, not for the reviewer: I'd like to emphasize that the primary purpose of this change is NOT to avoid the linker error. As commented in the ticket, to prvent or mitigate this type of trouble, we should provide a more generic and comprehensive defense, such as creating a separate install directory, rather than tweaking conflicting library names one by one.

comment:3 Changed 9 years ago by jinmei

  • Component changed from Unclassified to DNSPacket API
  • Milestone set to 05. 3rd Incremental Release: Serious Secondary
  • Owner set to UnAssigned
  • Status changed from new to reviewing

comment:4 Changed 9 years ago by jreed

  • Owner changed from UnAssigned to jreed

comment:5 follow-up: Changed 9 years ago by jreed

This looks correct and works for me.

My only comment is maybe the src/lib/dns directory could be renamed to src/lib/dns++ to be consistent.

comment:6 in reply to: ↑ 5 Changed 9 years ago by jinmei

Replying to jreed:

This looks correct and works for me.

My only comment is maybe the src/lib/dns directory could be renamed to src/lib/dns++ to be consistent.

Yeah, I thought about that, but was not sure if it's really a good idea.

First, within BIND 10 there's no ambiguity whether we say "dns" or "dns++", so the latter is a bit redundant. Also, "+" requires a shift key to type in many keyboards, and I'm afraid it may make developers irritated if they find they need to push the shift key every time they try to open a file under the libdns++ module (I'm half joking, but half serious, and I know completion may help).

Besides, as for consistency, we are already not so consistent: the library name under src/lib/config is libcfgclient (although renaming this may be less controversial).

So I'm going to commit this branch as is, and then send a note about the directory name to the dev list in case someone has a strong opinion or compelling argument for either way (I suspect not, though).

comment:7 Changed 9 years ago by jinmei

  • Resolution set to fixed
  • Status changed from reviewing to closed

merged, r2153. closing.

Note: See TracTickets for help on using tickets.