unofficial mirror of notmuch@notmuchmail.org
 help / color / Atom feed
* [PATCH] util/zlib-extra: de-inline gzerror_str
@ 2020-04-27 12:28 David Bremner
  2020-04-27 16:54 ` Daniel Kahn Gillmor
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: David Bremner @ 2020-04-27 12:28 UTC (permalink / raw)
  To: notmuch

It turns out the behaviour of inline functions in C header files is
not a good idea, and can cause linking problems if the compiler
decides not to inline them.  In principle this is solvable by using a
"static inline" declaration, but this potentially makes a copy in
every compilation unit. Since we don't actually care about the
performance of this function, just use a non-inline function.
---
 util/zlib-extra.c | 7 +++++++
 util/zlib-extra.h | 4 ++--
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/util/zlib-extra.c b/util/zlib-extra.c
index 2d2d2414..3a75e504 100644
--- a/util/zlib-extra.c
+++ b/util/zlib-extra.c
@@ -85,3 +85,10 @@ gz_error_string (util_status_t status, gzFile file)
     else
 	return util_error_string (status);
 }
+
+const char *
+gzerror_str(gzFile file)
+{
+    int dummy;
+    return gzerror (file, &dummy);
+}
diff --git a/util/zlib-extra.h b/util/zlib-extra.h
index 296dc914..e9925c98 100644
--- a/util/zlib-extra.h
+++ b/util/zlib-extra.h
@@ -29,8 +29,8 @@ gz_error_string (util_status_t status, gzFile stream);
 
 /* Call gzerror with a dummy errno argument, the docs don't promise to
  * support the NULL case */
-inline const char *
-gzerror_str(gzFile file) { int dummy; return gzerror (file, &dummy); }
+const char *
+gzerror_str(gzFile file);
 
 #ifdef __cplusplus
 }
-- 
2.26.2

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] util/zlib-extra: de-inline gzerror_str
  2020-04-27 12:28 [PATCH] util/zlib-extra: de-inline gzerror_str David Bremner
@ 2020-04-27 16:54 ` Daniel Kahn Gillmor
  2020-04-27 18:03 ` Tomi Ollila
  2020-04-28 13:38 ` David Bremner
  2 siblings, 0 replies; 4+ messages in thread
From: Daniel Kahn Gillmor @ 2020-04-27 16:54 UTC (permalink / raw)
  To: David Bremner, notmuch


[-- Attachment #1.1: Type: text/plain, Size: 544 bytes --]

On Mon 2020-04-27 09:28:08 -0300, David Bremner wrote:
> It turns out the behaviour of inline functions in C header files is
> not a good idea, and can cause linking problems if the compiler
> decides not to inline them.  In principle this is solvable by using a
> "static inline" declaration, but this potentially makes a copy in
> every compilation unit. Since we don't actually care about the
> performance of this function, just use a non-inline function.

LGTM.  No need for premature optimization in the error case anyway.

        --dkg

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]



^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] util/zlib-extra: de-inline gzerror_str
  2020-04-27 12:28 [PATCH] util/zlib-extra: de-inline gzerror_str David Bremner
  2020-04-27 16:54 ` Daniel Kahn Gillmor
@ 2020-04-27 18:03 ` Tomi Ollila
  2020-04-28 13:38 ` David Bremner
  2 siblings, 0 replies; 4+ messages in thread
From: Tomi Ollila @ 2020-04-27 18:03 UTC (permalink / raw)
  To: David Bremner, notmuch

On Mon, Apr 27 2020, David Bremner wrote:

> It turns out the behaviour of inline functions in C header files is
> not a good idea, and can cause linking problems if the compiler
> decides not to inline them.  In principle this is solvable by using a
> "static inline" declaration, but this potentially makes a copy in
> every compilation unit. Since we don't actually care about the
> performance of this function, just use a non-inline function.

LGTM.

Tomi

> ---
>  util/zlib-extra.c | 7 +++++++
>  util/zlib-extra.h | 4 ++--
>  2 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/util/zlib-extra.c b/util/zlib-extra.c
> index 2d2d2414..3a75e504 100644
> --- a/util/zlib-extra.c
> +++ b/util/zlib-extra.c
> @@ -85,3 +85,10 @@ gz_error_string (util_status_t status, gzFile file)
>      else
>  	return util_error_string (status);
>  }
> +
> +const char *
> +gzerror_str(gzFile file)
> +{
> +    int dummy;
> +    return gzerror (file, &dummy);
> +}
> diff --git a/util/zlib-extra.h b/util/zlib-extra.h
> index 296dc914..e9925c98 100644
> --- a/util/zlib-extra.h
> +++ b/util/zlib-extra.h
> @@ -29,8 +29,8 @@ gz_error_string (util_status_t status, gzFile stream);
>  
>  /* Call gzerror with a dummy errno argument, the docs don't promise to
>   * support the NULL case */
> -inline const char *
> -gzerror_str(gzFile file) { int dummy; return gzerror (file, &dummy); }
> +const char *
> +gzerror_str(gzFile file);
>  
>  #ifdef __cplusplus
>  }
> -- 
> 2.26.2

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] util/zlib-extra: de-inline gzerror_str
  2020-04-27 12:28 [PATCH] util/zlib-extra: de-inline gzerror_str David Bremner
  2020-04-27 16:54 ` Daniel Kahn Gillmor
  2020-04-27 18:03 ` Tomi Ollila
@ 2020-04-28 13:38 ` David Bremner
  2 siblings, 0 replies; 4+ messages in thread
From: David Bremner @ 2020-04-28 13:38 UTC (permalink / raw)
  To: notmuch

David Bremner <david@tethera.net> writes:

> It turns out the behaviour of inline functions in C header files is
> not a good idea, and can cause linking problems if the compiler
> decides not to inline them.  In principle this is solvable by using a
> "static inline" declaration, but this potentially makes a copy in
> every compilation unit. Since we don't actually care about the
> performance of this function, just use a non-inline function.

pushed to master, with slightly tweaked commit message

d

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-27 12:28 [PATCH] util/zlib-extra: de-inline gzerror_str David Bremner
2020-04-27 16:54 ` Daniel Kahn Gillmor
2020-04-27 18:03 ` Tomi Ollila
2020-04-28 13:38 ` David Bremner

unofficial mirror of notmuch@notmuchmail.org

Archives are clonable:
	git clone --mirror https://yhetil.org/notmuch/0 notmuch/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 notmuch notmuch/ https://yhetil.org/notmuch \
		notmuch@notmuchmail.org
	public-inbox-index notmuch

Example config snippet for mirrors

Newsgroups are available over NNTP:
	nntp://news.yhetil.org/yhetil.mail.notmuch.general
	nntp://news.gmane.io/gmane.mail.notmuch.general


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git