sezero / mikmod

Mikmod Sound System (mirror of git repo at https://sf.net/projects/mikmod/)
http://mikmod.sourceforge.net/
69 stars 21 forks source link

Fix misc. warnings in MikMod reported by GCC 12.2.1 #73

Closed AliceLR closed 1 year ago

AliceLR commented 1 year ago

Please CAREFULLY review and test these warning fixes for the MikMod player. They seem fine to me and I encountered no issues in the affected parts of the UI.

Side note: for affected old MSVCs and MINGWs without the stdio wrappers for snprintf, the macro SNPRINTF should probably have a safety wrapper. Same for HAVE_VSNPRINTF, but also the case when HAVE_VSNPRINTF isn't defined looks really bad.

sezero commented 1 year ago

marchive.c:280: strncpy warning should have been suppressed but wasn't.

Two other uses of strncpy in filename2short versions at lines 252 and 281 need the same change.

mconfedit.c:374: egregious usage of strcat/strncat.

This gives me headaches, but should be OK I think

mwindow.c:612: bizarre bounding of strncpy length on the source string which is better suited to a function that doesn't suck (snprintf).

If msg is NULL, the result will be undefined behavior: https://stackoverflow.com/questions/11589342

AliceLR commented 1 year ago

Two other uses of strncpy in filename2short versions at lines 252 and 281 need the same change.

Done.

If msg is NULL, the result will be undefined behavior: https://stackoverflow.com/questions/11589342

I think the diff obfuscated this, but it is still guarded by if (msg) { ... }.

sezero commented 1 year ago

If msg is NULL, the result will be undefined behavior: https://stackoverflow.com/questions/11589342

I think the diff obfuscated this, but it is still guarded by if (msg) { ... }.

I was confused by the fact that there is no else case anymore but the code keeps calculating strlen(status_message) down below: but that is what the old code did, yes? In that case, I guess it's OK.

sezero commented 1 year ago

This is in now. Thanks.