Fix the non-compiling utmp case, found on OpenBSD. #57
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Good call, updated the commit message. |
DerDakon
approved these changes
Aug 6, 2019
|
For the benefit of readers not in front of an OpenBSD box, will you explain operationally what "fix" you needed because this change was "non-compiling?" You have access to a lot more information that I would myself like to be privileged to--will you amend the commit message explaining what happened rather than what you did? |
|
The macro did not properly escape all continuation lines. |
Even saying that much is an improvement over the current message, agreed. It also left out a temporary variable. |
By not having build-tested with utmp, I made two easy mistakes:
(1) In file included from qbiff.c:3:
./qtmp.h:15:5: error: unknown type name 'substdio'
substdio ssutmp; \
^
./qtmp.h:16:43: error: expected ';' after top level declarator
char bufutmp[sizeof(struct utmp) * 16]
^
;
Mistake: missed a trailing backslash on line 14.
Fix: add it.
(2) qbiff.c:81:2: error: use of undeclared identifier 'fdutmp'; did you mean 'ssutmp'?
UTMP_OPEN;
^
./qtmp.h:20:33: note: expanded from macro 'UTMP_OPEN'
substdio_fdbuf(&ssutmp,read,fdutmp,bufutmp,sizeof(bufutmp))
Mistake: tried to get rid of fdutmp by checking open_read() directly,
not realizing fdutmp was still needed on the next line (!).
Fix: bring back fdutmp.
Fixes: 47baf78
alanpost
suggested changes
Aug 6, 2019
alanpost
approved these changes
Aug 6, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
By not having build-tested with utmp, I made two easy mistakes:
(1) In file included from qbiff.c:3:
./qtmp.h:15:5: error: unknown type name 'substdio'
substdio ssutmp;
^
./qtmp.h:16:43: error: expected ';' after top level declarator
char bufutmp[sizeof(struct utmp) * 16]
^
;
Mistake: missed a trailing backslash on line 14.
Fix: add it.
(2) qbiff.c:81:2: error: use of undeclared identifier 'fdutmp'; did you mean 'ssutmp'?
UTMP_OPEN;
^
./qtmp.h:20:33: note: expanded from macro 'UTMP_OPEN'
substdio_fdbuf(&ssutmp,read,fdutmp,bufutmp,sizeof(bufutmp))
Mistake: tried to get rid of fdutmp by checking open_read() directly,
not realizing fdutmp was still needed on the next line (!).
Fix: bring back fdutmp.