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

RE: [PATCH v2 4/5] cmocka: Use cast_ptr_to_largest_integral_type in suitable places



> -----Original Message-----
> From: Andreas Schneider [mailto:asn@xxxxxxxxxxxxxx]
> Sent: Friday, February 13, 2015 1:53 PM
> To: Krzysztof Opasiak
> Cc: jakub.hrozek@xxxxxxxxx; cmocka@xxxxxxxxxxxxxx; Pawel Szewczyk;
> Stanislaw Wadas
> Subject: Re: [PATCH v2 4/5] cmocka: Use
> cast_ptr_to_largest_integral_type in suitable places
> 
> On Friday 13 February 2015 11:30:19 Krzysztof Opasiak wrote:
> > > -----Original Message-----
> > > From: Andreas Schneider [mailto:asn@xxxxxxxxxxxxxx]
> > > Sent: Thursday, February 12, 2015 5:38 PM
> > > To: Krzysztof Opasiak
> > > Cc: jakub.hrozek@xxxxxxxxx; cmocka@xxxxxxxxxxxxxx;
> > > p.szewczyk@xxxxxxxxxxx; s.wadas@xxxxxxxxxxx
> > > Subject: Re: [PATCH v2 4/5] cmocka: Use
> > > cast_ptr_to_largest_integral_type in suitable places
> > >
> > > On Thursday 12 February 2015 14:11:40 Krzysztof Opasiak wrote:
> > > > Replace cast_to_largest_integral_type() with its ptr
> > > > versions in macros which are supposed to take pointer
> > > > as their parameter.
> > >
> > > I don't think this is correct. We had a bug in this area. If
> you
> > > are on 32bit
> > > and have a 64bit variable and cast it as a ptr which is 32bit
> you
> > > will loose
> > > data.
> >
> > I'm not sure if it's a bug because if you look a few lines above
> you
> > will find
> > Function signature used when generating doxygen:
> >
> > void will_return(#function, void *value);
> >
> > So according to documentation users should not abuse this macro
> to pass
> > values
> > Which cannot be fit in sizeof(void *)*8 bits.
> >
> > But of course it's only my opinion and you have the right to
> decide how
> > it should be done;)
> 
> 
> Check the master branch documentation :) Let me know if this is
> fine for you.
> 
> 

Ok, why not, looks good to me.

Please tell me, why do you use such ugly tricks with largest integer
type and casting ptr instead of defining a union?

You could simply do:

typedef union {
	void  *ptr;
	LargestIntegerType integer;
	float f;
	double d;
} ArgumentContent;

typedef struct {
	enum ArgumentType type;
	ArgumentContent content;
} Argument;


Cheers,
Krzysztof