Skip to content

libobs/util: Fix os_process_pipe_create on Linux #12049

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

franga2000
Copy link

Description

This fixes a regression on Linux, introduced in commit 9bc3082.

According to the POSIX specification for the exec family of commands, "The first argument is the filename or pathname of the executable to be executed". This was done correctly before, but the above commit removed "sh" from the arguments, breaking the pipe function on Linux.

Motivation and Context

In OBS 31.0.3 on GNU/Linux, the os_process_pipe_create function fails silently, as the /bin/sh shipped with most Linux distros "eats" the first argument and so no program is launched.

How Has This Been Tested?

Compiled and used for a day in production. Given that this is essentially a revert of one line back to 30.1.0, you could say it has been tested quite extensively.

OS: Arch Linux
Used plugins: websocket, NDI, obs-ssp (my fork, this is where I discovered the issue)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • My code has been run through clang-format.
  • I have read the contributing document.
  • My code is not on the master branch.
  • The code has been tested.
  • All commit messages are properly formatted and commits squashed where appropriate.
  • I have included updates to all appropriate documentation.

This fixes a regression on Linux, introduced in commit 9bc3082.

According to the POSIX specification for the exec family of commands, 
"The first argument is the filename or pathname of the executable to 
be executed". This was done correctly before, but the above commit 
removed "sh" from the arguments, breaking the pipe function on Linux.
@norihiro
Copy link
Contributor

LGTM.

Tested with a code below on Fedora 40.

#include <stdio.h>
#include "libobs/util/pipe.h"

int main()
{
	os_process_pipe_t *pp = os_process_pipe_create("echo Success", "r");

	char buf[128] = {0};
	os_process_pipe_read(pp, buf, sizeof(buf) - 1);

	printf("Output: '%s'\n", buf);

	int ret = os_process_pipe_destroy(pp);

	printf("Return: %d\n", ret);

	return ret;

}

Output on the master branch (looks bad):

Output: ''
Return: 127

Output on the master branch (looks good):

Output: 'Success
'
Return: 0

@RytoEX RytoEX requested a review from derrod April 14, 2025 21:12
@RytoEX RytoEX added Bug Fix Non-breaking change which fixes an issue Linux Affects Linux labels Apr 14, 2025
@PatTheMav
Copy link
Member

This change also affects macOS, but it is indeed required to pass the file name(!) of the binary as argv[0] to posix_spawn (which is also the minimum required content of that array).

@RytoEX RytoEX added the macOS Affects macOS label Apr 18, 2025
Copy link
Member

@RytoEX RytoEX left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks correct. @derrod ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Fix Non-breaking change which fixes an issue Linux Affects Linux macOS Affects macOS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants