Re: [Rhythmbox-devel] DACP (iTunes remote) support added

[Alexandre Rosenfeld <alexandre rosenfeld gmail com>]

On Mon, Oct 18, 2010 at 17:01, Bastien Nocera <hadess hadess net> wrote:

On Mon, 2010-10-18 at 16:00 -0500, W. Michael Petullo wrote:
> > I'm building it on a 32 bit system and got this error:
> >
> >   CC     libdaap_la-rb-daap-record.lo
> > cc1: warnings being treated as errors
> > rb-daap-record.c: In function ‘rb_daap_record_new’:
> > rb-daap-record.c:413: error: cast from pointer to integer of different size
> >
> > The following one line fix makes it compile:
> >
> > $ git diff
> > diff --git a/plugins/daap/rb-daap-record.c b/plugins/daap/rb-daap-record.c
> > index fee6064..d8e5740 100644
> > --- a/plugins/daap/rb-daap-record.c
> > +++ b/plugins/daap/rb-daap-record.c
> > @@ -409,7 +409,7 @@ RBDAAPRecord *rb_daap_record_new (RhythmDBEntry *entry)
> >                                                 (entry, RHYTHMDB_PROP_ALBUM);
> >
> >                 /* Since we don't support album id's on Rhythmbox,
> > "emulate" it */
> > -               record->priv->albumid  = (gint64) rhythmdb_entry_get_refstring
> > +               record->priv->albumid  = (gint) rhythmdb_entry_get_refstring
> >                                                 (entry, RHYTHMDB_PROP_ALBUM);
> >
> >                 record->priv->genre    = rhythmdb_entry_dup_string
> >
> >
> > Is this a sane fix?
>
> But, int is 4 bytes on x86_64 and pointers are 8 bytes.
>
> I thought maybe:
>
> record->priv->albumid = GPOINTER_TO_UINT(rhythmdb_entry_get_refstring
>                                       (entry, RHYTHMDB_PROP_ALBUM));
>
> However, the documentation for GPOINTER_TO_UINT says, "Remember, YOU
> MAY NOT STORE POINTERS IN INTEGERS. THIS IS NOT PORTABLE IN ANY WAY
> SHAPE OR FORM. These macros ONLY allow storing integers in pointers,
> and only preserve 32 bits of the integer; values outside the range of
> a 32-bit integer will be mangled."
>
> Should we handle this with a pre-processor trick?

If you're hoping to use a pointer there, just make it a pointer, instead
of seemingly trying to work around it.

It's not a pointer, it's just a way to workaround the fact that Rhythmbox doesnt have id's for albums and DACP requires one, so we use the refstring pointer location (which should be constant through a Rhythmbox execution) to always get the same number for a string (in this case, an album name).

I thought the code would work because it would cast a 32-bit pointer (4 bytes) into a 64-bit integer in 32-bit machines. And in 64-bits it works normally. 

Any suggestion to make this work?

Btw, thanks Jonathan for reviewing and committing this code!

Regards,

Alexandre