Libvisual / libvisual

Libvisual Audio Visualization
http://libvisual.org/
82 stars 31 forks source link

[0.4.x] libvisual: fix accidental assignment instead of equality check #351

Open triallax opened 1 month ago

triallax commented 1 month ago

with the accidental assignment, the function would always return 0 for a keytype of VISUAL_HASHMAP_KEY_TYPE_STRING (because it has a value of 2), which is technically valid but not good for obvious reasons

kaixiong commented 1 month ago

Nice find. Neither Clang nor GCC warns about this assignment?

triallax commented 1 month ago

i don't know about gcc, but i found out about this because clang warned about it when i was bumping libvisual for chimera linux:

lv_hashmap.c:264:19: warning: using the result of an assignment as a condition without parentheses [-Wparentheses]
  264 |         else if (keytype = VISUAL_HASHMAP_KEY_TYPE_STRING)
      |                  ~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
lv_hashmap.c:264:19: note: place parentheses around the assignment to silence this warning
  264 |         else if (keytype = VISUAL_HASHMAP_KEY_TYPE_STRING)
      |                          ^                               
      |                  (                                       )
lv_hashmap.c:264:19: note: use '==' to turn this assignment into an equality comparison
  264 |         else if (keytype = VISUAL_HASHMAP_KEY_TYPE_STRING)
      |                          ^
      |                          ==
kaixiong commented 1 month ago

@kaixiong what do you think?

@hartwork would adding -Werror be a good idea? I haven't had to look at or build 0.4.x in a long time to tell.

hartwork commented 1 month ago

@kaixiong outside of configure.ac in CI that would be great — yes! —, but my memory says that we're not at zero with warnings and so we cannot enable it without fixing more warnings first. Would need a closer look.