A: broken linking (if VirtualBox linked against OpenSSL from ports) the port links against OpenSSL's libcrypto, but doesn't honor WITH_OPENSSL_PORT=yes At the same time it links to virtualbox/VBoxRT.so which is linked to either base's or port's OpenSSL's libcrypto if VBoxRT is linked to port's OpenSSL then istgt binary become linked to both BASE's and PORT's libcrypto It may cause program malfunction because of overlapping symbols and structures with identical name B: broken cast There are so many places with the construct like: ISTGT_WARNLOG("Connection reset by peer (%s,time=%d)\n", conn->initiator_name, (int)difftime(now, start)); cast from double to int is invalid althougth it raise warning only, resulting code may not work on some architectures at all C: broken compilation (just for completeness) it doesn't compile with VBOXVD option turned on at all firing error: /usr/ports/emulators/virtualbox-ose/work/VirtualBox-4.2.14/include/VBox/vd.h:182: error: expected specifier-qualifier-list before 'PARTITIONING_TYPE' Fix: A: 1. the libcrypto is used just for the purpose of calculation of MD5 digest. istgt needs to be linked against same version (base/ports) of libcrypto as virtualbox/VBoxRT.so has been linked to 2. dependency to port's OpenSSL should be recorded if istgt become linked to it Even if we modify the port to link against appropriate libcrypto and record dependency if necesarry, then administrator need to identify what version of libcrypto should be used or a complex autodetection logic needs to be implemented So I suggest different approach to avoid conflict. I replaced libcrypto's MD5_*() by libmd's MD5*(). No conflict may occur, no dependency needs to be recorded, no administrator decision nor autodetection logic necesarry at all. Required changes are small: a) MD5_Init/MD5_Update/MD5_Final functions are replaced by MD5Init/MD5Update/MD5Final b) openssl/md5.h header reference replaced by reference to sys/types.h and md5.h c) references to libcrypto replaced by reference to libmd. See patch-DAN-replacecrypto B: printf double as true double not int using %.0f format As construct in question are used for error logging only, not in casual code paths, such change should not affect the performance nor CPU utilisation See patch-DAN-invalidcast C: this bug is not in istgt code but bug in virtualbox header. But it doesn't affect compilation of VirtualBox itself. I include it just for completeness as istgt is only affected software known to me. Error is caused by PARTITIONING_TYPE symbol declared as enum in header file but referenced without enum keyword, e.g. like typedef's type. The symbol is not referenced as 'enum' in any place of istgt. Even VirtualBox code (not used during standard compilation of VirtualBox) references symbol without 'enum' keyword. I added typedef wrapper around it's declaration. It's conform to style of declaration of other enums and structures. I assume the wrapper has been forgotten here. See emulators/virtualbox-ose/files/patch-DAN-include-VBox-vd.h patch --- emulators/virtualbox-ose/files/patch-DAN-include-VBox-vd.h begins here --- How-To-Repeat: A: compile VirtualBox with WITH_OPENSSL_PORT=yes, then compile istgt see warning past linking command: cc -Wl,-rpath,/usr/local/lib/virtualbox -o istgtcontrol istgtcontrol.o istgt_conf.o istgt_log.o istgt_sock.o istgt_misc.o istgt_md5.o -lcam -lcrypto -lpthread /usr/local/lib/virtualbox/VBoxDDU.so /usr/local/lib/virtualbox/VBoxRT.so B: see several warnings during compilation: cast from function call of type 'double' to non-matching type 'int' C: turn VBOXVD option on. Compilation will abort with error mentioned in "Description"
Maintainer of net/istgt, Please note that PR ports/179989 has just been submitted. If it contains a patch for an upgrade, an enhancement or a bug fix you agree on, reply to this email stating that you approve the patch and a committer will take care of it. The full text of the PR can be found at: http://www.freebsd.org/cgi/query-pr.cgi?pr=ports/179989 -- Edwin Groothuis via the GNATS Auto Assign Tool edwin@FreeBSD.org
State Changed From-To: open->feedback Awaiting maintainers feedback (via the GNATS Auto Assign Tool)
Notifying maintainer again (aoyama@peach.ne.jp) as it seems that it was lost in GNAT conversion. aoyama, please respond to this old PR. What do you want to do about it?
I consider the maintainer has "timed out". Anybody can commit this.
FYI, I am also resetting the maintainer. This port should have been reset with the other unstaged ports, but apparently this PR block that reset.
A commit references this bug: Author: marino Date: Fri Aug 8 19:03:20 UTC 2014 New revision: 364376 URL: http://svnweb.freebsd.org/changeset/ports/364376 Log: net/istgt: Fix broken linking and casts While here, fix stage-qa warnings about unstripped binaries. Also the maintainer should have been reset due to lack of staging, but the presence of this PR prevented it. Due to this and the lack of response on this old PR, reset the maintainer too. PR: 179989 Submitted by: Dan Lukes Changes: head/net/istgt/Makefile head/net/istgt/files/patch-invalidcast head/net/istgt/files/patch-replacecrypto head/net/istgt/files/patch-src_Makefile.in
I obviously couldn't do anything with the third patch, that would require a separate PR. Sorry this took so long, better late than never I guess.
The "third patch" and "separate PR" you are writing about are https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=181324 :-) -- Martin