compel/test: Fix success condition check in fdspy

[ancientmodern]

Found this bug when working on adding riscv64 support to compel #1702 :) Specifically, the bug appeared when the p_err pipe became broken after resuming, causing check_pipe_ends() to hit either these branches:

if (write(wfd, "1", 2) != 2) {
	fprintf(stderr, "write to pipe failed\n");
	return -1;
}
if (read(rfd, aux, sizeof(aux)) != sizeof(aux)) {
	fprintf(stderr, "read from pipe failed\n");
	return -1;
}

The -1 return value was not correctly handled in the final check:

pass = check_pipe_ends(stolen_fd, p_err[0]);

if (pass)
	printf("All OK\n");
else
	printf("Something went WRONG\n");

This pull request modified the condition to pass == 1, ensuring that "All OK" is printed only when check_pipe_ends() returns 1 (indicating a successful case), and any other value will correctly result in "Something went WRONG" being printed.

[codecov-commenter]

Codecov Report

Patch coverage: 25.00% and project coverage change: +0.02 🎉

Comparison is base (4d137b8) 70.74% compared to head (581d7f6) 70.77%.

❗ Current head 581d7f6 differs from pull request most recent head 8e68170. Consider uploading reports for the commit 8e68170 to get more accurate results

Additional details and impacted files

@@             Coverage Diff              @@
##           criu-dev    #2192      +/-   ##
============================================
+ Coverage     70.74%   70.77%   +0.02%     
============================================
  Files           133      133              
  Lines         33244    33233      -11     
============================================
+ Hits          23518    23520       +2     
+ Misses         9726     9713      -13     

Impacted Files	Coverage Δ
criu/cgroup.c	74.53% <0.00%> (ø)
criu/include/restorer.h	100.00% <ø> (ø)
criu/include/rst_info.h	100.00% <ø> (ø)
criu/mem.c	86.30% <ø> (+0.95%)	⬆️
criu/memfd.c	81.81% <20.00%> (-1.26%)	⬇️
criu/cr-restore.c	67.19% <100.00%> (-0.19%)	⬇️

... and 3 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[mihalicyn]

Hi @ancientmodern !

Thanks for looking into it.

Speaking about RISC-V support, Yixue (@felicitia) working on that closely. Are your really working on that too? I think you need to contact Yixue then.

Now about your fix. As far as I understand this inconsistency appeared after this commit 5364ca3
IMHO, it's better to always return 0 in case of error (then you don't need to change if condition).

[rst0git]

As far as I understand this inconsistency appeared after this commit 5364ca3
IMHO, it's better to always return 0 in case of error (then you don't need to change if condition).

Thanks for pointing this out Alex! I agree, check_pipe_ends() should return 0 in the case of an error to be consistent with the other conditions in that function.

[ancientmodern]

Hi @ancientmodern !

Thanks for looking into it.

Speaking about RISC-V support, Yixue (@felicitia) working on that closely. Are your really working on that too? I think you need to contact Yixue then.

Now about your fix. As far as I understand this inconsistency appeared after this commit 5364ca3 IMHO, it's better to always return 0 in case of error (then you don't need to change if condition).

Thank you for the feedback Alex! Yeah I've contacted Yixue and we are working together on RISC-V support. As per Yixue's recent update in #1702, we've successfully enabled most of compel to function on RISC-V, and we are actively addressing a few remaining bugs :)

I agree it's better to consistently return 0 in case of error and will make the changes. As this is a fix to a previous commit, do I need to follow the guideline about "fix tags" mentioned here?

Edit: I've updated my commit. Please let me know if there is any issue I need to resolve :)

[rst0git]

LGTM