search

solusipse / fiche

Command line pastebin for sharing terminal output.
http://termbin.com
MIT License
1.41k stars 166 forks source link

Remove the VLA from handle_connection() #109

Open mixi opened 3 years ago

mixi commented 3 years ago

This fixes a segfault on musl libc with reasonable sized buffers, as musl's default thread stack size is quite small (128k since 1.1.21).

A similar bug exists on glibc with large enough buffers (reproducable with e.g. 16MB on my test system).

This fixes #108.

mixi commented 3 years ago

I also now implemented the memory savings on glibc by requesting a reasonably small thread stack as proposed in #108.

dashezup commented 3 years ago

it only receives 8 bytes on https://github.com/solusipse/fiche/pull/109/commits/f3c7a296d779935b39c9d5b7f9bb9b5208dc8c0b (tested on musl), no matter if -B is set or not.

============================================================
[Fiche][STATUS] Sat Apr  3 10:09:30 2021
[Fiche][STATUS] Incoming connection from: 127.0.0.1 (localhost.localdomain).
[Fiche][STATUS] Received 8 bytes, saved to: hmxd.
============================================================

or it's still wip? @ me when you need me to test it.

mixi commented 3 years ago

@dashezup: That was me missing that sizeof(buffer) was being used to determine the size of the buffer (works with a static buffer / a VLA, doesn't work with a dynamically allocated buffer). It should be fixed now.

dashezup commented 3 years ago

Nice :+1: , I've tested https://github.com/solusipse/fiche/commit/fcfcff954d697fcfe03804a3acc7d83006d6ae45 on musl, it works properly, I can upload very huge text (like 1G, probably memory was full and can't add more in that case)

mixi commented 3 years ago

That was me changing the 16k of requested thread stack to the musl default of 128k. That should probably be safer and still less than the glibc default.