Skip to content

Conversation

@lalten
Copy link
Contributor

@lalten lalten commented Apr 3, 2022

This PR adds code to check the return code of the mksquashfs process. appimagekit will now die if mksquashfs failed.

Copy link
Member

@TheAssassin TheAssassin left a comment

Choose a reason for hiding this comment

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

Thanks, looks good overall. See my comment, applies to all perror calls.


if (pid == -1) {
// error, failed to fork()
perror("fork()");
Copy link
Member

Choose a reason for hiding this comment

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

Please add some context information. We use other subprocesses, too, within the scope of AppImageKit. Ideally, the message informs the user that this was trying to open the mksquashfs process.

@lalten lalten requested a review from TheAssassin April 5, 2022 12:56
@lalten
Copy link
Contributor Author

lalten commented Aug 18, 2022

ping on this one as well @TheAssassin :)

@TheAssassin
Copy link
Member

Will have a look. Haven't had time for AppImage related projects in a while, and when things broke I had to invest like a full week or maybe even two...


int retcode = WEXITSTATUS(status);
if (retcode) {
fprintf(stderr, "mksquashfs (pid %ld) exited with code %d\n", (long)pid, retcode);
Copy link
Member

Choose a reason for hiding this comment

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

pid_t should be an int in glibc, so I'm not sure this cast is necessary, %d should work. (After all, long int and int are most likely the same size (4 bytes) on every viable platform anyway...)

@lalten
Copy link
Contributor Author

lalten commented Sep 20, 2022

Friendly ping on this one, @TheAssassin :)

@TheAssassin TheAssassin force-pushed the die-on-mksquashfs-failure branch from e20c676 to f7f00bc Compare September 29, 2022 12:45
@TheAssassin TheAssassin merged commit b719a7f into AppImage:master Sep 29, 2022
@TheAssassin
Copy link
Member

Thanks!

@lalten lalten deleted the die-on-mksquashfs-failure branch September 29, 2022 20:52
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.

2 participants