lkml.org 
[lkml]   [2017]   [Jan]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 56/62] watchdog: tangox_wdt: Convert to use device managed functions
On Thu, Jan 12, 2017 at 10:44:07AM +0100, Marc Gonzalez wrote:
> On 11/01/2017 18:51, Guenter Roeck wrote:
>
> > However, some other unrelated undefined behavior does not mean that this
> > specific behavior is undefined.
>
> True :-)
>
> Let me just give two additional examples of UB that /have/ bitten
> Linux kernel devs.
>
> int i;
> for (i = 1; i > 0; ++i)
> /* do_something(); */
>
> => optimized into an infinite loop
>
> and
>
> void func(struct foo *p) {
> int n = p->field;
> if (!p) return;
>
> => null-pointer check optimized away
>
> > So far we have a claim that a cast to a void * may somehow be different
> > to a cast to a different pointer, if used as function argument, and that
> > the behavior with such a cast may be undefined. In other words, you claim
> > that a function implemented as, say,
> >
> > void func(int *var) {}
> >
> > might result in undefined behavior if some header file declares it as
> >
> > void func(void *);
> >
> > and it is called as
> >
> > int var;
> >
> > func(&var);
> >
> > That seems really far fetched to me.
>
> Thanks for giving me an opportunity to play the language lawyer :-)
>
> C99 6.3.2.3 sub-clause 8 states:
>
> "A pointer to a function of one type may be converted to a pointer to a function of another
> type and back again; the result shall compare equal to the original pointer. If a converted
> pointer is used to call a function whose type is not compatible with the pointed-to type,
> the behavior is undefined."
>
> So, the behavior is undefined, not when you cast clk_disable_unprepare,
> but when clk_disable_unprepare is later called through the devres->action
> function pointer.
>
> However, I agree that it will work as expected on typical platforms
> (where all pointers are the same size, and the calling convention
> treats all pointers the same).
>
> > I do get the message that you do not like this kind of cast. But that doesn't
> > mean it is not correct.
>
> If it's already widely used in the kernel, it seems there is no point
> fighting it ;-)

I'd say +.5 here (where +1 is an ack). My approach would be to push
devm_clk_prepare_enable and use that. It cannot be that hard, can it?

It looks prettier, is well defined, easier to fit into 80 chars per
line. I wonder why not everybody jubilates on this new function.

Best regards
Uwe

--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |

\
 
 \ /
  Last update: 2017-01-12 10:58    [W:0.192 / U:4.768 seconds]
©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site