RE: [PATCH 0/5] cmocka: Make casting macros public

[Krzysztof Opasiak <k.opasiak@xxxxxxxxxxx>]

Hi,

> -----Original Message-----
> From: Jakub Hrozek [mailto:jakub.hrozek@xxxxxxxxx]
> Sent: Wednesday, February 11, 2015 7:54 PM
> To: Krzysztof Opasiak
> Cc: asn@xxxxxxxxxxxxxx; cmocka@xxxxxxxxxxxxxx;
> p.szewczyk@xxxxxxxxxxx; s.wadas@xxxxxxxxxxx
> Subject: Re: [PATCH 0/5] cmocka: Make casting macros public
> 
> 
> 
> On 11.02.2015 18:58, Krzysztof Opasiak wrote:
> > Dear Andreas and Jakub,
> >
> > I'm using cmocka to wrap file system operations. Some functions,
> > for example fopen() takes path as its argument. In linux path
> > elements can be separated using unlimited number of slashes.
> > So, for example:
> >
> > /my/home/dir
> > //my//////home////dir
> >
> > points to the same directory and both are correct. To check
> > if argument which contains path is correct we have implemented
> > our own comparison function and use expect_check() macro to
> > store its value. To use this macro, our function should take
> > LargestIntegralType parameter. We need to cast this int to
> > const char * but it causes ugly warnings:
> >
> > warning: cast to pointer from integer of different size
> > [-Wint-to-pointer-cast]
> >
> > After reading cmocka source I have found some macros which
> > does exactly the job. My commit makes those macros available
> > for all cmocka users by moving their definitions to public
> > headers.
> >
> 
> First of all, thank you very much for the patches.
> 
> > First commit helps doxygen generate more suitable documentation
> > by adding code section around include suggestions.
> >
> 
> ACK
> 
> > Second commit takes all the ifndef stuff from cmocka.h to
> > separate header to improve cmocka API readability.
> >
> 
> I couldn't apply this commit on master, can you rebase? Chances are
> we
> pushed quite a lot of stuff today...a git tree is fine as well for
> testing...
> 
> But after scrolling through the diff, I don't see any issues. I'm
> not
> sure I like the cmocka_extra.h name but I can't think of a better
> one
> either.. (cmocka_ptr? cmocka_platform? dunno)

I also have no idea what would be the most suitable name. I have pick
this
One as there is a lot of defines and other stuff which is hard to
categorize.
Up to this I pick name suggested by Andreas.

> 
> The vast majority of users wouldn't even notice this split, since
> 
> > Third commit make casting macros public to fix issues described
> > above.
> >
> 
> LGTM, not tested since I couldn't apply the patches
> 
> > Fourth commit replace usage of cast_to_largest_integral_type()
> > with cast_ptr_to_largest_integral_type() in places where macros
> > are supposed to take pointer as its parameter.
> >
> 
> I would prefer if Andreas reviewed this one, he did fixes precisely
> in
> this area today.
> 
> > Last one is a small fix of examples to use correct assert macros
> > make them simpler and avoid compiler warnings.
> >
> 
> Visual ACK, but I couldn't apply the patch.

Patches has been rebased onto current HEAD
(328288bd88a043cf2fefcd10f1efd892414f4a11)
So they should apply now.

Thank you very much for your review and very fast responses;)


Cheers,

Krzysztof