On 2018-07-10 08:24, Leon Romanovsky wrote:
> On Tue, Jul 10, 2018 at 08:09:32AM +0200, Rasmus Villemoes wrote:
>> On 2018-07-08 12:38, Leon Romanovsky wrote:
>>
>>> +#define check_shift_overflow(a, s, d) ({ \
>>> + typeof(a) _a = a; \
>>> + typeof(s) _s = s; \
>>> + typeof(d) _d = d; \
>>> + u64 _a_full = _a; \
>>> + unsigned int _to_shift = \
>>> + _s >= 0 && _s < 8 * sizeof(*d) ? _s : 0; \
>>> + *_d = (_a_full << _to_shift); \
>>> + *d = *_d; \
>>
>> As others have pointed out, this assignment is useless at best, but
>> potentially harmful - the whole point of the standard typeof dance is to
>> prevent double evaluation of macro arguments...
>
> In the lib/test_overflow.c, there is expectation what result of
> check_one_op will be defined anyway (line 231).
You _do_ know how pointers work in C, right? *_d = ... already initializes whatever _d
points to, and that's exactly the same thing that d points to - unless of course the
expression d has some side effect that makes it point somewhere else between the two
expansions. So either *d = *_d is a pointless self-assignment, or it's plain buggy because
the side effects of d happen twice, and we've now probably written to at least one
location we weren't supposed to write to...
> I wanted a way to have
> overflowed result placed in *d for tests, but didn't want for users rely
> on that, so this is why comment says that the result is unreliable.
Yeah, even if that was a valid reason (see the other reply), *d _is_ already
initialized-in-the-C-sense. It's just that we don't promise any particular contents.
Rasmus
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
