From d7d18e4e4328badea85d40be16cbdb59adbac337 Mon Sep 17 00:00:00 2001 From: Kalle Olavi Niemitalo Date: Sat, 28 Mar 2009 20:15:08 +0200 Subject: [PATCH] bug 1047: inline functions C99 conformance C99 6.7.4p3 and 6.7.4p6 set some constraints on what can be done in inline functions and how they can be declared. In particular, any function declared inline must also be defined in the same translation unit. To comply with that, remove inline specifiers from function declarations in header files when the functions are not also defined in those header files. Sun Studio 11 on Solaris 9 is stricter than C99 and does not allow references to static identifiers in extern inline functions. Make the configure script detect this and define NONSTATIC_INLINE accordingly in config.h. Then use that in the definitions of all non-static inline functions. Document the restrictions and this scheme in doc/hacking.txt. --- NEWS | 3 +-- configure.in | 23 +++++++++++++++++++++ doc/hacking.txt | 45 ++++++++++++++++++++++++++++++++++++++++++ src/document/options.c | 2 +- src/intl/charsets.c | 26 ++++++++++++++++++------ src/intl/charsets.h | 12 +++++------ src/osdep/stub.c | 22 ++++++++++----------- src/osdep/stub.h | 4 ++-- src/session/session.c | 2 +- src/terminal/color.c | 2 +- src/terminal/color.h | 4 ++-- src/terminal/draw.c | 2 +- src/terminal/tab.c | 2 +- src/util/conv.c | 4 ++-- src/util/string.c | 14 ++++++------- 15 files changed, 124 insertions(+), 43 deletions(-) diff --git a/NEWS b/NEWS index c2620b6c..dd65db96 100644 --- a/NEWS +++ b/NEWS @@ -9,8 +9,7 @@ ELinks 0.12pre2.GIT now: ------------------------ To be released as 0.12pre3, 0.12rc1, or even 0.12.0. This branch also -includes the changes listed under ``ELinks 0.11.6'' below, except the -fix for bug 1047. +includes the changes listed under ``ELinks 0.11.6'' below. Incompatibilities: diff --git a/configure.in b/configure.in index ee8a382e..490cc12f 100644 --- a/configure.in +++ b/configure.in @@ -225,6 +225,29 @@ AC_SUBST(CONFIG_INTERLINK) AC_STRUCT_TM AC_C_CONST AC_C_INLINE +AC_MSG_CHECKING([[C99-conforming inline]]) +AC_COMPILE_IFELSE([[ + int add(int x); + + static int sum; + + inline int + add(int change) + { + sum += change; + return sum; + } + + int + sub(int change) + { + return add(-change); + }]], + [AC_MSG_RESULT([[yes]]) + AC_DEFINE([NONSTATIC_INLINE], [inline], + [Define as inline if the compiler lets you declare a function without inline, then define it with inline, and have that definition refer to identifiers with internal linkage. This is allowed by C99 6.7.4p6 and 6.7.4p3 together. Otherwise define as nothing.])], + [AC_MSG_RESULT([[no]]) + AC_DEFINE([NONSTATIC_INLINE], [])]) EL_CHECK_CODE(typeof, HAVE_TYPEOF, [], [int a; typeof(a) b;]) AC_SYS_LARGEFILE diff --git a/doc/hacking.txt b/doc/hacking.txt index 2cd107b2..7b43afe7 100644 --- a/doc/hacking.txt +++ b/doc/hacking.txt @@ -246,6 +246,51 @@ remarkable implication of no C99 are no C++ (//) comments and declarations at the start of block only. +Inline functions +---------------- + +Various standards and compilers set restrictions on inline functions: + +- C89 doesn't support them at all. + +- According to C99 6.7.4p6, if a translation unit declares a function + as inline and not static, then it must also define that function. + +- According to C99 6.7.4p3, an inline definition of a non-static + function must not contain any non-const static variables, and must + not refer to anything static outside of it. According to C99 + 6.7.4p6 however, if the function is ever declared without inline or + with extern, then its definition is not an inline definition even if + the definition includes the inline keyword. + +- Sun Studio 11 on Solaris 9 does not let non-static inline functions + refer to static identifiers. Unlike C99 6.7.4p3, this seems to + apply to all definitions of inline functions, not only inline + definitions. See ELinks bug 1047. + +- GCC 4.3.1 warns if a function is declared inline after being called. + Perhaps it means such calls cannot be inlined. + +- In C99, a function definition with extern inline means the compiler + should inline calls to that function and must also emit out-of-line + code that other translation units can call. In GCC 4.3.1, it + instead means the out-of-line definition is elsewhere. + +So to be portable to all of those, we use inline functions in only +these ways: + +- Define as static inline in a *.c file. Perhaps declare in that same + file. On C89, we #define inline to nothing. + +- Define as static inline in a *.h file. Perhaps declare in that same + file. On C89, we #define inline to nothing, and extra copies of the + function may then get emitted but anyway it'll work. + +- Declare without inline in a *.h file, and define with NONSTATIC_INLINE + in a *.c file. Perhaps also declare with NONSTATIC_INLINE in the same + *.c file. On Sun Studio 11, we #define NONSTATIC_INLINE to nothing. + + Comments -------- diff --git a/src/document/options.c b/src/document/options.c index 8160c493..a9eb94dd 100644 --- a/src/document/options.c +++ b/src/document/options.c @@ -104,7 +104,7 @@ compare_opt(struct document_options *o1, struct document_options *o2) && o1->box.width != o2->box.width); } -inline void +NONSTATIC_INLINE void copy_opt(struct document_options *o1, struct document_options *o2) { copy_struct(o1, o2); diff --git a/src/intl/charsets.c b/src/intl/charsets.c index 9acffb9c..86c6f488 100644 --- a/src/intl/charsets.c +++ b/src/intl/charsets.c @@ -69,6 +69,19 @@ struct codepage_desc { #include "intl/uni_7b.inc" #include "intl/entity.inc" +/* Declare the external-linkage inline functions defined in this file. + * Avoid the GCC 4.3.1 warning: `foo' declared inline after being + * called. The functions are not declared inline in charsets.h + * because C99 6.7.4p6 says that every external-linkage function + * declared inline shall be defined in the same translation unit. + * The non-inline declarations in charsets.h also make sure that the + * compiler emits global definitions for the symbols so that the + * functions can be called from other translation units. */ +NONSTATIC_INLINE unsigned char *encode_utf8(unicode_val_T u); +NONSTATIC_INLINE int utf8charlen(const unsigned char *p); +NONSTATIC_INLINE int unicode_to_cell(unicode_val_T c); +NONSTATIC_INLINE unicode_val_T utf8_to_unicode(unsigned char **string, + const unsigned char *end); static const char strings[256][2] = { "\000", "\001", "\002", "\003", "\004", "\005", "\006", "\007", @@ -212,7 +225,7 @@ u2cp_(unicode_val_T u, int to, enum nbsp_mode nbsp_mode) static unsigned char utf_buffer[7]; -inline unsigned char * +NONSTATIC_INLINE unsigned char * encode_utf8(unicode_val_T u) { memset(utf_buffer, 0, 7); @@ -261,12 +274,13 @@ static const char utf8char_len_tab[256] = { }; #ifdef CONFIG_UTF8 -inline int utf8charlen(const unsigned char *p) +NONSTATIC_INLINE int +utf8charlen(const unsigned char *p) { return p ? utf8char_len_tab[*p] : 0; } -inline int +int strlen_utf8(unsigned char **str) { unsigned char *s = *str; @@ -287,7 +301,7 @@ strlen_utf8(unsigned char **str) /* Start from @current and move back to @pos char. This pointer return. The * most left pointer is @start. */ -inline unsigned char * +unsigned char * utf8_prevchar(unsigned char *current, int pos, unsigned char *start) { if (current == NULL || start == NULL || pos < 0) @@ -582,7 +596,7 @@ invalid_arg: * TODO: May be extended to return 0 for zero-width glyphs * (like composing, maybe unprintable too). */ -inline int +NONSTATIC_INLINE int unicode_to_cell(unicode_val_T c) { if (c >= 0x1100 @@ -625,7 +639,7 @@ unicode_fold_label_case(unicode_val_T c) } #endif /* CONFIG_UTF8 */ -inline unicode_val_T +NONSTATIC_INLINE unicode_val_T utf8_to_unicode(unsigned char **string, const unsigned char *end) { unsigned char *str = *string; diff --git a/src/intl/charsets.h b/src/intl/charsets.h index 8bcb0533..32a676fe 100644 --- a/src/intl/charsets.h +++ b/src/intl/charsets.h @@ -113,10 +113,10 @@ unsigned char *get_cp_config_name(int); unsigned char *get_cp_mime_name(int); int is_cp_utf8(int); void free_conv_table(void); -inline unsigned char *encode_utf8(unicode_val_T); +unsigned char *encode_utf8(unicode_val_T); #ifdef CONFIG_UTF8 -inline unsigned char *utf8_prevchar(unsigned char *, int, unsigned char *); -inline int utf8charlen(const unsigned char *); +unsigned char *utf8_prevchar(unsigned char *, int, unsigned char *); +int utf8charlen(const unsigned char *); int utf8_char2cells(unsigned char *, unsigned char *); int utf8_ptr2cells(unsigned char *, unsigned char *); int utf8_ptr2chars(unsigned char *, unsigned char *); @@ -141,11 +141,11 @@ unsigned char *utf8_step_forward(unsigned char *, unsigned char *, int, enum utf8_step, int *); unsigned char *utf8_step_backward(unsigned char *, unsigned char *, int, enum utf8_step, int *); -inline int unicode_to_cell(unicode_val_T); +int unicode_to_cell(unicode_val_T); unicode_val_T unicode_fold_label_case(unicode_val_T); -inline int strlen_utf8(unsigned char **); +int strlen_utf8(unsigned char **); #endif /* CONFIG_UTF8 */ -inline unicode_val_T utf8_to_unicode(unsigned char **, const unsigned char *); +unicode_val_T utf8_to_unicode(unsigned char **, const unsigned char *); unicode_val_T cp_to_unicode(int, unsigned char **, const unsigned char *); unicode_val_T cp2u(int, unsigned char); diff --git a/src/osdep/stub.c b/src/osdep/stub.c index f8da1d4d..56489d6f 100644 --- a/src/osdep/stub.c +++ b/src/osdep/stub.c @@ -40,7 +40,7 @@ #define toupper_delta(s1, s2) (toupper(*((char *) s1)) - toupper(*((char *) s2))) #ifndef HAVE_STRCASECMP -inline int +NONSTATIC_INLINE int elinks_strcasecmp(const char *s1, const char *s2) { while (*s1 != '\0' && toupper_equal(s1, s2)) { @@ -53,7 +53,7 @@ elinks_strcasecmp(const char *s1, const char *s2) #endif /* !HAVE_STRCASECMP */ #ifndef HAVE_STRNCASECMP -inline int +NONSTATIC_INLINE int elinks_strncasecmp(const char *s1, const char *s2, size_t len) { if (len == 0) @@ -72,7 +72,7 @@ elinks_strncasecmp(const char *s1, const char *s2, size_t len) #ifndef HAVE_STRCASESTR /* Stub for strcasestr(), GNU extension */ -inline char * +NONSTATIC_INLINE char * elinks_strcasestr(const char *haystack, const char *needle) { size_t haystack_length = strlen(haystack); @@ -93,7 +93,7 @@ elinks_strcasestr(const char *haystack, const char *needle) #endif #ifndef HAVE_STRDUP -inline char * +NONSTATIC_INLINE char * elinks_strdup(const char *str) { int str_len = strlen(str); @@ -114,7 +114,7 @@ elinks_strdup(const char *str) extern int sys_nerr; extern const char *const sys_errlist[]; #endif -inline const char * +NONSTATIC_INLINE const char * elinks_strerror(int err_no) { if (err_no < 0 || err_no > sys_nerr) @@ -126,7 +126,7 @@ elinks_strerror(int err_no) #ifndef HAVE_STRSTR /* From http://www.unixpapa.com/incnote/string.html */ -inline char * +NONSTATIC_INLINE char * elinks_strstr(const char *s, const char *p) { char *sp, *pp; @@ -155,7 +155,7 @@ elinks_strstr(const char *s, const char *p) * arguments reversed. * From http://www.unixpapa.com/incnote/string.html */ /* Modified for ELinks by Zas. */ -inline void * +NONSTATIC_INLINE void * elinks_memmove(void *d, const void *s, size_t n) { register char *dst = (char *) d; @@ -178,7 +178,7 @@ elinks_memmove(void *d, const void *s, size_t n) #ifndef HAVE_STPCPY -inline char * +NONSTATIC_INLINE char * elinks_stpcpy(char *dest, const char *src) { while ((*dest++ = *src++)); @@ -187,7 +187,7 @@ elinks_stpcpy(char *dest, const char *src) #endif #ifndef HAVE_MEMPCPY -inline void * +NONSTATIC_INLINE void * elinks_mempcpy(void *dest, const void *src, size_t n) { return (void *) ((char *) memcpy(dest, src, n) + n); @@ -195,7 +195,7 @@ elinks_mempcpy(void *dest, const void *src, size_t n) #endif #ifndef HAVE_ISDIGIT -inline int +NONSTATIC_INLINE int elinks_isdigit(int i) { return i >= '0' && i <= '9'; @@ -203,7 +203,7 @@ elinks_isdigit(int i) #endif #ifndef HAVE_MEMRCHR -inline void * +NONSTATIC_INLINE void * elinks_memrchr(const void *s, int c, size_t n) { char *pos = (char *) s; diff --git a/src/osdep/stub.h b/src/osdep/stub.h index 8236a24f..00986472 100644 --- a/src/osdep/stub.h +++ b/src/osdep/stub.h @@ -54,7 +54,7 @@ #ifndef HAVE_ISDIGIT #undef isdigit #define isdigit(a) elinks_isdigit(a) -inline int elinks_isdigit(int); +int elinks_isdigit(int); #endif /** strerror() */ @@ -78,7 +78,7 @@ char *elinks_strstr(const char *, const char *); #else #undef memmove #define memmove(dst, src, n) elinks_memmove(dst, src, n) -inline void *elinks_memmove(void *, const void *, size_t); +void *elinks_memmove(void *, const void *, size_t); #endif #endif diff --git a/src/session/session.c b/src/session/session.c index 0b4ff8c7..f024e8be 100644 --- a/src/session/session.c +++ b/src/session/session.c @@ -428,7 +428,7 @@ load_ecmascript_imports(struct session *ses, struct document_view *doc_view) #define load_ecmascript_imports(ses, doc_view) #endif -inline void +NONSTATIC_INLINE void load_frames(struct session *ses, struct document_view *doc_view) { struct document *document = doc_view->document; diff --git a/src/terminal/color.c b/src/terminal/color.c index e0e5b715..9b13ac6d 100644 --- a/src/terminal/color.c +++ b/src/terminal/color.c @@ -241,7 +241,7 @@ static const unsigned char fg_color[16][8] = { #define CMPCODE(c) (((c) << 1 | (c) >> 2) & TERM_COLOR_MASK) #define use_inverse(bg, fg) CMPCODE(fg & TERM_COLOR_MASK) < CMPCODE(bg) -inline void +NONSTATIC_INLINE void set_term_color16(struct screen_char *schar, enum color_flags flags, unsigned char fg, unsigned char bg) { diff --git a/src/terminal/color.h b/src/terminal/color.h index 4ce8d31b..e466bf7a 100644 --- a/src/terminal/color.h +++ b/src/terminal/color.h @@ -63,8 +63,8 @@ enum color_mode { COLOR_MODES = 5, /* XXX: Keep last */ }; -inline void set_term_color16(struct screen_char *schar, enum color_flags flags, - unsigned char fg, unsigned char bg); +void set_term_color16(struct screen_char *schar, enum color_flags flags, + unsigned char fg, unsigned char bg); /** Mixes the color pair and attributes to a terminal text color. * If @a flags has masked in the ::COLOR_INCREASE_CONTRAST the diff --git a/src/terminal/draw.c b/src/terminal/draw.c index 6deb72a0..b3b3706f 100644 --- a/src/terminal/draw.c +++ b/src/terminal/draw.c @@ -33,7 +33,7 @@ -inline struct screen_char * +NONSTATIC_INLINE struct screen_char * get_char(struct terminal *term, int x, int y) { assert(term && term->screen && term->screen->image); diff --git a/src/terminal/tab.c b/src/terminal/tab.c index b9795483..d0c25a63 100644 --- a/src/terminal/tab.c +++ b/src/terminal/tab.c @@ -62,7 +62,7 @@ found_pos: } /** Number of tabs at the terminal (in term->windows) */ -inline int +NONSTATIC_INLINE int number_of_tabs(struct terminal *term) { int result = 0; diff --git a/src/util/conv.c b/src/util/conv.c index 0404a3ac..35ac2568 100644 --- a/src/util/conv.c +++ b/src/util/conv.c @@ -50,7 +50,7 @@ * * @returns 0 if OK or width needed for the whole number to fit there, * if it had to be truncated. A negative value signs an error. */ -int inline +NONSTATIC_INLINE int elinks_ulongcat(unsigned char *s, unsigned int *slen, unsigned long number, unsigned int width, unsigned char fillchar, unsigned int base, @@ -108,7 +108,7 @@ elinks_ulongcat(unsigned char *s, unsigned int *slen, } /** Similar to elinks_ulongcat() but for @c long number. */ -int inline +NONSTATIC_INLINE int elinks_longcat(unsigned char *s, unsigned int *slen, long number, unsigned int width, unsigned char fillchar, unsigned int base, diff --git a/src/util/string.c b/src/util/string.c index f101245f..604a00d7 100644 --- a/src/util/string.c +++ b/src/util/string.c @@ -297,7 +297,7 @@ char * c_strcasestr(const char *haystack, const char *needle) /* TODO Currently most of the functions use add_bytes_to_string() as a backend * instead we should optimize each function. */ -inline struct string * +NONSTATIC_INLINE struct string * #ifdef DEBUG_MEMLEAK init_string__(const unsigned char *file, int line, struct string *string) #else @@ -322,7 +322,7 @@ init_string(struct string *string) return string; } -inline void +NONSTATIC_INLINE void done_string(struct string *string) { assertm(string != NULL, "[done_string]"); @@ -341,7 +341,7 @@ done_string(struct string *string) } /** @relates string */ -inline struct string * +NONSTATIC_INLINE struct string * add_to_string(struct string *string, const unsigned char *source) { assertm(string && source, "[add_to_string]"); @@ -355,7 +355,7 @@ add_to_string(struct string *string, const unsigned char *source) } /** @relates string */ -inline struct string * +NONSTATIC_INLINE struct string * add_crlf_to_string(struct string *string) { assertm(string != NULL, "[add_crlf_to_string]"); @@ -374,7 +374,7 @@ add_crlf_to_string(struct string *string) } /** @relates string */ -inline struct string * +NONSTATIC_INLINE struct string * add_string_to_string(struct string *string, const struct string *from) { assertm(string && from, "[add_string_to_string]"); @@ -451,7 +451,7 @@ string_concat(struct string *string, ...) } /** @relates string */ -inline struct string * +NONSTATIC_INLINE struct string * add_char_to_string(struct string *string, unsigned char character) { assertm(string && character, "[add_char_to_string]"); @@ -468,7 +468,7 @@ add_char_to_string(struct string *string, unsigned char character) return string; } -inline struct string * +NONSTATIC_INLINE struct string * add_xchar_to_string(struct string *string, unsigned char character, int times) { int newlength;