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

request.c: Use errno only if it is non-zero (fixes #893) #1009

Merged
merged 1 commit into from
Jul 24, 2024

Conversation

zdohnal
Copy link
Member

@zdohnal zdohnal commented Jul 12, 2024

In case of internal errors, we use errno for generating error message, but there are cases (like operation timeout for listing drivers or models) when errno is zero.
We should take error from http->error if errno is zero, falling back to Internal Server Error if even http->error is 0 and still we have HTTP_STATUS_ERROR.

Fixes #893

Copy link
Member

@michaelrsweet michaelrsweet left a comment

Choose a reason for hiding this comment

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

See comments. I think we need a little code refactoring here…

cups/request.c Outdated Show resolved Hide resolved
cups/request.c Outdated Show resolved Hide resolved
@zdohnal zdohnal force-pushed the use-errno-only-if-set branch 3 times, most recently from 8562930 to fa94eab Compare July 16, 2024 06:31
Copy link
Member

@michaelrsweet michaelrsweet left a comment

Choose a reason for hiding this comment

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

See comments.

cups/http-addr.c Outdated Show resolved Hide resolved
@zdohnal zdohnal force-pushed the use-errno-only-if-set branch 2 times, most recently from a339565 to b7855aa Compare July 22, 2024 08:45
Copy link
Member

@michaelrsweet michaelrsweet left a comment

Choose a reason for hiding this comment

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

A few small nits - see comments.

cups/request.c Outdated Show resolved Hide resolved
cups/request.c Outdated Show resolved Hide resolved
Sometimes errno is not set when we want to report HTTP error, so we
should use `http->error` if available or internal server error. In cases
of internal HTTP related errors where we don't have HTTP connection
available (before setting of HTTP connection or in callbacks which
process IPP messages), use `_cupsSetError()`.

Fixes OpenPrinting#893
Copy link
Member

@michaelrsweet michaelrsweet left a comment

Choose a reason for hiding this comment

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

+1

@zdohnal zdohnal merged commit c88ee33 into OpenPrinting:master Jul 24, 2024
2 of 6 checks passed
@andi34
Copy link

andi34 commented Aug 7, 2024

Currently breaks the build

ipp.c: In function ‘ippReadIO’:
ipp.c:3211:1: error: invalid use of void expression
 3211 | +             _cupsSetError(IPP_STATUS_ERROR_INTERNAL, _("Unable to allocate IPP attribute."), 1);
      | ^
make[1]: *** [../Makedefs:271: ipp.o] Error 1
make[1]: Leaving directory '/home/runner/work/cups/cups/cups'
make: *** [Makefile:43: all] Error 1

@zdohnal
Copy link
Member Author

zdohnal commented Aug 8, 2024

@andi34 ouch! Thanks! I was too eager there, it should be fixed now.

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.

libcups returns success if scheduler does not send data in time (f.e. cups-driverd times out)
3 participants