Re: [PATCH v3 1/1] Makefile: link libcurl before libssl

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Remi Pommarel wrote:

> For static linking especially library order while linking is important. For

Please be more explicit for the uninitiated: during static linking, the
library that needs symbols comes before the library providing those symbols.

> example, libcurl wants symbols for libssl when building http-push, http-fetch
> and remote-curl. So for these programs libcurl has to be linked before libssl.

Based on experience with NEEDS_CRYPTO_WITH_SSL vs NEEDS_SSL_WITH_CRYPTO,
these kinds of dependencies can be platform-specific.  But a little quick
thought reveals that libssl is never going to depend on libcurl and libcurl
always needs libssl.  Good.

Based on this dependency, shouldn't CURL_LIBCURL always include -lssl when
statically linking?  How does this relate to NEEDS_SSL_WITH_CURL?

> --- a/Makefile
> +++ b/Makefile
> @@ -1029,7 +1029,6 @@ ifdef HAVE_ALLOCA_H
>  endif
>  
>  IMAP_SEND_BUILDDEPS =
> -IMAP_SEND_LDFLAGS = $(OPENSSL_LINK) $(OPENSSL_LIBSSL) $(LIB_4_CRYPTO)

To protect against a value that might leak in from the environment, this
should say

	IMAP_SEND_LDFLAGS =

[...]
> @@ -1971,10 +1971,10 @@ git-imap-send$X: imap-send.o $(IMAP_SEND_BUILDDEPS) GIT-LDFLAGS $(GITLIBS)
>  
>  git-http-fetch$X: http.o http-walker.o http-fetch.o GIT-LDFLAGS $(GITLIBS)
>  	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
> -		$(LIBS) $(CURL_LIBCURL)
> +		$(CURL_LIBCURL) $(LIBS)

What happens in the NEEDS_SSL_WITH_CURL=Yes case?

>  git-http-push$X: http.o http-push.o GIT-LDFLAGS $(GITLIBS)
>  	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
> -		$(LIBS) $(CURL_LIBCURL) $(EXPAT_LIBEXPAT)
> +		$(CURL_LIBCURL) $(EXPAT_LIBEXPAT) $(LIBS)

Same question.

I guess the general principle is that $(LIBS) should always go at the
end.  (That would make sense to me.)

Ideally this would be two patches:

 - one putting $(LIBS) at the end everywhere, which is the simple part of the change
 - a second doing some appropriate thing to turn on NEEDS_SSL_WITH_CURL when appropriate
   or something

Sensible?

Thanks,
Jonathan
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]