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
Fix regression from commit 165330b7bf0757e30fa8a6de9998a564fb62796f #1545
Conversation
With the changed logic, the if-clause can fall through without ever initializing s. The exit code condition is getting more complicated now so move it to helper variable, assume failure for a safe default. Fixes: 165330b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch! I should have used -Werror=maybe-uninitialized
here. We can use a ternary conditional to make the code a bit shorter.
@@ -409,7 +409,8 @@ unsigned headerSizeof(Header h, int magicp) | |||
static inline int strtaglen(const char *str, rpm_count_t c, const char *end) | |||
{ | |||
const char *start = str; | |||
const char *s; | |||
const char *s = NULL; | |||
int len = -1; /* assume failure */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
int len = -1; /* assume failure */ |
if (s != NULL && c == 0) | ||
len = s - str + 1; | ||
|
||
return len; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (s != NULL && c == 0) | |
len = s - str + 1; | |
return len; | |
return s != NULL && c == 0 ? s - str + 1 : -1; |
Note the commit message:
Shortness is only a goal if it helps readability. |
Indeed that is true. Another option would have been to re-introduce the earlier check, but this is more readable, IMO. |
With the changed logic, the if-clause can fall through without ever
initializing s. The exit code condition is getting more complicated
now so move it to helper variable, assume failure for a safe default.
Fixes: 165330b