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

Ensure the return value from get_server_path is freeable #2276

Conversation

frankleonrose
Copy link
Contributor

While testing in support of #2252 I ran into this crash bug which looks like this:

% x/app/scrcpy
2021-04-22 12:50:31.596 scrcpy[82192:39073792] INFO: scrcpy 1.16 <https://github.com/Genymobile/scrcpy>
2021-04-22 12:50:31.596 scrcpy[82192:39073792] ERROR: Could not get executable path, using scrcpy-server from current directory
scrcpy(82192,0x11122fdc0) malloc: *** error for object 0x10a439c2c: pointer being freed was not allocated
scrcpy(82192,0x11122fdc0) malloc: *** set a breakpoint in malloc_error_break to debug
scrcpy-server: 1 file pushed. 4.5 MB/s (33622 bytes in 0.007s)
zsh: abort      x/app/scrcpy --window-borderless

The fix is simply that in the case where get_server_path returns just SERVER_FILENAME, it must be duplicated, because the caller expects to be able to free the returned value. If the duplication fails, we return NULL which indicates failure, so no further error handling is necessary, unless we wanted to log something about that extremely rare situation.

@rom1v
Copy link
Collaborator

rom1v commented Apr 22, 2021

Yes, thank you 👍 LGTM.

However, I'm surprised you execute this code. Did you explicitly enabled -Dportable=true in your meson command?

rom1v pushed a commit that referenced this pull request Apr 22, 2021
PR #2276 <#2276>

Signed-off-by: Romain Vimont <rom@rom1v.com>
@rom1v
Copy link
Collaborator

rom1v commented Apr 22, 2021

I rebased on dev (so I also replaced SDL_strdup() by strdup(), because of changes on that branch, and merged: aaf7875

@rom1v rom1v closed this Apr 22, 2021
@frankleonrose
Copy link
Contributor Author

Yes, I did specify portable because I was testing the 1.16 version and had the compatible server local.

Thanks for taking the fix. Sorry I hadn't targeted dev.

@rom1v
Copy link
Collaborator

rom1v commented Apr 23, 2021

I did specify portable because I was testing the 1.16 version and had the compatible server local.

OK. Just for info, instead, you could just use -Dprebuilt_server=... in your meson command, or set SCRCPY_SERVER_PATH at runtime.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants