cursor: Fix issue with gcc

Submitted by Stefan Dirsch on April 21, 2016, 3:45 p.m.

Details

Message ID 1461253524-14517-1-git-send-email-sndirsch@suse.de
State New
Series "cursor: Fix issue with gcc"
Headers show

Commit Message

Stefan Dirsch April 21, 2016, 3:45 p.m.
From: Kristoffer Grönlund <krig@koru.se>

cursor_shape_to_id should not be declared const, else it is not added
to the static library.
---
 cursor/cursor.h          | 2 +-
 cursor/shape_to_id.gperf | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

Patch hide | download patch | download mbox

diff --git a/cursor/cursor.h b/cursor/cursor.h
index 455dc34..3fe7096 100644
--- a/cursor/cursor.h
+++ b/cursor/cursor.h
@@ -161,7 +161,7 @@  typedef struct xcint_image_t {
 } __attribute__((packed)) xcint_image_t;
 
 /* shape_to_id.c */
-const int cursor_shape_to_id(const char *name);
+int cursor_shape_to_id(const char *name);
 
 /* parse_cursor_file.c */
 int parse_cursor_file(xcb_cursor_context_t *c, const int fd, xcint_image_t **images, int *nimg);
diff --git a/cursor/shape_to_id.gperf b/cursor/shape_to_id.gperf
index 23615f9..6c8818e 100644
--- a/cursor/shape_to_id.gperf
+++ b/cursor/shape_to_id.gperf
@@ -1,5 +1,5 @@ 
 struct shape_mapping { const char *name; int number; };
-const int cursor_shape_to_id(const char *name);
+int cursor_shape_to_id(const char *name);
 %%
 X_cursor,0
 arrow,1
@@ -79,7 +79,7 @@  ur_angle,74
 watch,75
 xterm,76
 %%
-const int cursor_shape_to_id(const char *name) {
+int cursor_shape_to_id(const char *name) {
 	struct shape_mapping *mapping = in_word_set(name, strlen(name));
 	return (mapping ? (mapping->number * 2) : -1);
 }

Comments

Uli Schlachter April 24, 2016, 6:47 p.m.
CC'ing Michael Stapelberg, since this is "mostly his library".

Am 21.04.2016 um 17:45 schrieb Stefan Dirsch:
> From: Kristoffer Grönlund <krig@koru.se>
> 
> cursor_shape_to_id should not be declared const, else it is not added
> to the static library.

Huh? Why not? What does GCC do different for static libraries? What does GCC
even have to do with libraries at all (instead of e.g. the linker)? What does
"not addeed to the static library" mean? What exactly are you doing and what's
the error message?

(Note that static libraries contain .o files and not functions...)

Also: "Fix issue with gcc": Sorry, but my gcc works fine here, could you be more
specific.

> ---
>  cursor/cursor.h          | 2 +-
>  cursor/shape_to_id.gperf | 4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/cursor/cursor.h b/cursor/cursor.h
> index 455dc34..3fe7096 100644
> --- a/cursor/cursor.h
> +++ b/cursor/cursor.h
> @@ -161,7 +161,7 @@ typedef struct xcint_image_t {
>  } __attribute__((packed)) xcint_image_t;
>  
>  /* shape_to_id.c */
> -const int cursor_shape_to_id(const char *name);
> +int cursor_shape_to_id(const char *name);
>  
>  /* parse_cursor_file.c */
>  int parse_cursor_file(xcb_cursor_context_t *c, const int fd, xcint_image_t **images, int *nimg);
> diff --git a/cursor/shape_to_id.gperf b/cursor/shape_to_id.gperf
> index 23615f9..6c8818e 100644
> --- a/cursor/shape_to_id.gperf
> +++ b/cursor/shape_to_id.gperf
> @@ -1,5 +1,5 @@
>  struct shape_mapping { const char *name; int number; };
> -const int cursor_shape_to_id(const char *name);
> +int cursor_shape_to_id(const char *name);
>  %%
>  X_cursor,0
>  arrow,1
> @@ -79,7 +79,7 @@ ur_angle,74
>  watch,75
>  xterm,76
>  %%
> -const int cursor_shape_to_id(const char *name) {
> +int cursor_shape_to_id(const char *name) {
>  	struct shape_mapping *mapping = in_word_set(name, strlen(name));
>  	return (mapping ? (mapping->number * 2) : -1);
>  }
>
Kristoffer Grönlund April 25, 2016, 8:45 a.m.
Hi,

On Sun, Apr 24, 2016 at 8:47 PM, Uli Schlachter <psychon@znc.in> wrote:
> CC'ing Michael Stapelberg, since this is "mostly his library".
>
> Am 21.04.2016 um 17:45 schrieb Stefan Dirsch:
>> From: Kristoffer Grönlund <krig@koru.se>
>>
>> cursor_shape_to_id should not be declared const, else it is not added
>> to the static library.
>
> Huh? Why not? What does GCC do different for static libraries? What does GCC
> even have to do with libraries at all (instead of e.g. the linker)? What does
> "not addeed to the static library" mean? What exactly are you doing and what's
> the error message?
>
> (Note that static libraries contain .o files and not functions...)
>
> Also: "Fix issue with gcc": Sorry, but my gcc works fine here, could you be more
> specific.

I wish I could, but I created that patch in 2013 and I no longer have
any memory nor any local trace of what version of GCC I was using. My
guess however is that it is a more recent version of GCC than what you
are using.

Here's my wild guess as to what was going on: If all parameters to the
function are const and the return value is also const, it seems that
at least on the OBS at that time, GCC would decide to inline the
function out of existence, which led to the linker complaining that
the function doesn't exist.

I think that I encountered this while building awesome [1], if that helps.

[1]: https://awesome.naquadah.org/

Cheers,
Kristoffer
Stefan Dirsch April 25, 2016, 10:58 a.m.
On Mon, Apr 25, 2016 at 10:45:17AM +0200, Kristoffer Grönlund wrote:
> >> cursor_shape_to_id should not be declared const, else it is not added
> >> to the static library.
> >
> > Huh? Why not? What does GCC do different for static libraries? What does GCC
> > even have to do with libraries at all (instead of e.g. the linker)? What does
> > "not addeed to the static library" mean? What exactly are you doing and what's
> > the error message?
> >
> > (Note that static libraries contain .o files and not functions...)
> >
> > Also: "Fix issue with gcc": Sorry, but my gcc works fine here, could you be more
> > specific.
> 
> I wish I could, but I created that patch in 2013 and I no longer have
> any memory nor any local trace of what version of GCC I was using. My
> guess however is that it is a more recent version of GCC than what you
> are using.
> 
> Here's my wild guess as to what was going on: If all parameters to the
> function are const and the return value is also const, it seems that
> at least on the OBS at that time, GCC would decide to inline the
> function out of existence, which led to the linker complaining that
> the function doesn't exist.
> 
> I think that I encountered this while building awesome [1], if that helps.
> 
> [1]: https://awesome.naquadah.org/

Kristoffer, since we don't provide a static lib for libxcb-cursor (did we
really at that time?), I suggest to remove this patch from our package sources
and no longer pursue to bring it upstream. Is this ok for you?

Thanks,
Stefan

Public Key available
------------------------------------------------------
Stefan Dirsch (Res. & Dev.)   SUSE LINUX GmbH
Tel: 0911-740 53 0            Maxfeldstraße 5
FAX: 0911-740 53 479          D-90409 Nürnberg
http://www.suse.de            Germany 
---------------------------------------------------------------
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham
Norton, HRB 21284 (AG Nürnberg)
---------------------------------------------------------------
Kristoffer Grönlund April 25, 2016, 11:02 a.m.
Yes, that is fine. I didn't think this patch was a permanent fix, it was a
workaround for an issue I had while initially packaging. If the problem
shows up again I'll get back in touch.

Cheers,
Kristoffer

On Mon, Apr 25, 2016, 12:58 Stefan Dirsch <sndirsch@suse.de> wrote:

> On Mon, Apr 25, 2016 at 10:45:17AM +0200, Kristoffer Grönlund wrote:
> > >> cursor_shape_to_id should not be declared const, else it is not added
> > >> to the static library.
> > >
> > > Huh? Why not? What does GCC do different for static libraries? What
> does GCC
> > > even have to do with libraries at all (instead of e.g. the linker)?
> What does
> > > "not addeed to the static library" mean? What exactly are you doing
> and what's
> > > the error message?
> > >
> > > (Note that static libraries contain .o files and not functions...)
> > >
> > > Also: "Fix issue with gcc": Sorry, but my gcc works fine here, could
> you be more
> > > specific.
> >
> > I wish I could, but I created that patch in 2013 and I no longer have
> > any memory nor any local trace of what version of GCC I was using. My
> > guess however is that it is a more recent version of GCC than what you
> > are using.
> >
> > Here's my wild guess as to what was going on: If all parameters to the
> > function are const and the return value is also const, it seems that
> > at least on the OBS at that time, GCC would decide to inline the
> > function out of existence, which led to the linker complaining that
> > the function doesn't exist.
> >
> > I think that I encountered this while building awesome [1], if that
> helps.
> >
> > [1]: https://awesome.naquadah.org/
>
> Kristoffer, since we don't provide a static lib for libxcb-cursor (did we
> really at that time?), I suggest to remove this patch from our package
> sources
> and no longer pursue to bring it upstream. Is this ok for you?
>
> Thanks,
> Stefan
>
> Public Key available
> ------------------------------------------------------
> Stefan Dirsch (Res. & Dev.)   SUSE LINUX GmbH
> Tel: 0911-740 53 0            Maxfeldstraße 5
> FAX: 0911-740 53 479          D-90409 Nürnberg
> http://www.suse.de            Germany
> ---------------------------------------------------------------
> SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham
> Norton, HRB 21284 (AG Nürnberg)
> ---------------------------------------------------------------
>
Stefan Dirsch April 25, 2016, 12:02 p.m.
On Mon, Apr 25, 2016 at 11:02:08AM +0000, Kristoffer Grönlund wrote:
> Yes, that is fine. I didn't think this patch was a permanent fix, it was a
> workaround for an issue I had while initially packaging. If the problem shows
> up again I'll get back in touch.

It's removed now.

@xcb ML: Sorry for the noise!

Thanks,
Stefan

Public Key available
------------------------------------------------------
Stefan Dirsch (Res. & Dev.)   SUSE LINUX GmbH
Tel: 0911-740 53 0            Maxfeldstraße 5
FAX: 0911-740 53 479          D-90409 Nürnberg
http://www.suse.de            Germany 
---------------------------------------------------------------
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham
Norton, HRB 21284 (AG Nürnberg)
---------------------------------------------------------------