-
Notifications
You must be signed in to change notification settings - Fork 111
Check for empty patch filenames. #156
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
Conversation
quic-bjorande
left a comment
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.
Took me a while to figure out that we indeed have an issue here.
If the "filename" attribute is either empty or absent, attr_as_string() returns NULL, which will trip us on the strcmp() below. By including such details to your explanation, this would have been obvious - and I would have been more eager to merge the previous PR you had.
That said, what does it mean when a patch tag has an empty filename? Today we're crashing on them, with this change we're going to ignore them. But does the "filename" actually impact the patch?
Is it correct to ignore patch tags without filename, or is the "filename" attribute optional?
283d013 to
6f63cea
Compare
andersson
left a comment
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.
s/inlcude/include/ in commit message.
Per my previous comment, I think it would be nice to change "handle that scenario" to explicitly state that we're ignoring those entries. (And I still would like to know if that is the expected way to handle this long term)
6f63cea to
b195cdf
Compare
b195cdf to
6eaae75
Compare
andersson
left a comment
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.
Please run "make check" and make sure it passes cleanly.
Please also add a statement in the commit message, saying that this behavior aligns with how other Qualcomm tools handles this.
6eaae75 to
7986d35
Compare
Some device builds include patches with empty filenames. This change adds a check to ignore those patches and prevent segmentation faults. This approach ensures consistency with other Qualcomm tools, which also ignore such patches. Signed-off-by: Yvonne Kaire <[email protected]>
7986d35 to
a3436e2
Compare
igoropaniuk
left a comment
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.
LGTM
Some device builds inlcude empty filenames. This change adds a check to handle that scenario and avoid segmentation faults.