irixxxx / picodrive

Fast MegaDrive/MegaCD/32X emulator
Other
47 stars 24 forks source link

remove fflush() call before exit() #129

Closed ToKe79 closed 1 month ago

ToKe79 commented 1 month ago

Hello, attached is a patch that adds a cast to expected type to fix this compile error with gcc14 when building libretro core:

pico/carthw/svp/compiler.c: In function 'ssp_translate_block':
pico/carthw/svp/compiler.c:1800:24: error: passing argument 1 of 'rfflush' from incompatible pointer type [-Wincompatible-pointer-types]
 1800 |                 fflush(stdout);
      |                        ^~~~~~
      |                        |
      |                        FILE *
In file included from ./pico/pico_port.h:12,
                 from ./pico/pico_int.h:15,
                 from pico/carthw/svp/compiler.c:9:
platform/libretro/libretro-common/include/streams/file_stream_transforms.h:89:25: note: expected 'RFILE *' but argument is of type 'FILE *'
   89 | int64_t rfflush(RFILE * stream);
      |                 ~~~~~~~~^~~~~~
irixxxx commented 1 month ago

I was about to remove the fflush altogether. It's not needed since exit() will flush standard file descriptors anyway. So, if you remove do it I'll pull it in.

ToKe79 commented 1 month ago

no problem, I removed fflush(stdout|stderr) from the emulator code. I left fflush() in where it made sense:

gizmondo - logging https://github.com/irixxxx/picodrive/blob/695e6de85e06d47043003664531fc491eaa91240/platform/gizmondo/giz.c#L44

libretro - vfs: https://github.com/irixxxx/picodrive/blob/695e6de85e06d47043003664531fc491eaa91240/platform/libretro/libretro-common/vfs/vfs_implementation.c#L764

external conversion tool: https://github.com/irixxxx/picodrive/blob/695e6de85e06d47043003664531fc491eaa91240/tools/bin_to_cso_mp3/bin_to_cso_mp3.c#L456 https://github.com/irixxxx/picodrive/blob/695e6de85e06d47043003664531fc491eaa91240/tools/bin_to_cso_mp3/bin_to_cso_mp3.c#L517

file operation: https://github.com/irixxxx/picodrive/blob/695e6de85e06d47043003664531fc491eaa91240/zlib/gzio.c#L760

If you are okay with that, or want to remove more, let me know. As final step I should probably squash these and rename the PR.

irixxxx commented 1 month ago

hmm that's probably a bit too drastic. Let me do some review on the locations you worked on.

ToKe79 commented 1 month ago

sorry about that, you probably meant altogether from that spot, not altogether from the codebase. it makes sense to remove it right before exit(), but keep it where it is logging, so the buffer is displayed earlier. I will put those back except the one before exit() in compiler.c.

irixxxx commented 1 month ago

No rush. Let me first look at the code.

irixxxx commented 1 month ago

After review I think it's indeed sufficient to just remove the one fflush in pico/carthw/svp/compiler.c. All others are either in debug code or in other platforms and should probably stay.

ToKe79 commented 1 month ago

odd. I do not see the commit in the history.

irixxxx commented 1 month ago

It's in the release-testing branch which will become the new master branch rsn.