Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

-Wshift-negative-value in drivers/thermal/rcar_gen3_thermal.c #531

Open
Nathan-Huckleberry opened this issue Jun 12, 2019 · 3 comments

Comments

@Nathan-Huckleberry
Copy link

commented Jun 12, 2019

drivers/thermal/rcar_gen3_thermal.c:147:33: warning: shifting a negative signed value is undefined [-Wshift-negative-value]
                     / (ptat[0] - ptat[2])) + FIXPT_INT(TJ_3);
                                              ^~~~~~~~~~~~~~~
drivers/thermal/rcar_gen3_thermal.c:126:29: note: expanded from macro 'FIXPT_INT'
#define FIXPT_INT(_x) ((_x) << FIXPT_SHIFT)
                       ~~~~ ^
drivers/thermal/rcar_gen3_thermal.c:150:18: warning: shifting a negative signed value is undefined [-Wshift-negative-value]
                                 tsc->tj_t - FIXPT_INT(TJ_3));
                                 ~~~~~~~~~~~~^~~~~~~~~~~~~~~~

Literally just the value -41 being shifted left by 7. Could probably just be replaced by a constant or -41 could be changed to 4294967255U.

/* no idea where these constants come from */
#define TJ_3 -41
#define FIXPT_INT(_x) ((_x) << FIXPT_SHIFT)
@nickdesaulniers

This comment has been minimized.

Copy link
Member

commented Jun 12, 2019

drivers/thermal/rcar_gen3_thermal.c
125:#define FIXPT_SHIFT 7

That comment doesn't inspire confidence in what the correct value is, and it's not clear to me how to fix.

564e73d had 2 magic constants. I haven't found yet which commit removed one of them (maybe a similar fix is needed for TJ_3?).

Maybe the author's & maintainer's can clarify?

@Nathan-Huckleberry

This comment has been minimized.

Copy link
Author

commented Jun 13, 2019

Introduced in 4eb39f7. Looks like the sign was swapped on (ptat[0] - ptat[2])) + FIXPT_INT(TJ_3);. Should probably make TJ_3 positive 41 and update usages.

@Nathan-Huckleberry

This comment has been minimized.

Copy link
Author

commented Jun 13, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.