Skip to content

Fix error checking fbnetboot/main.go#2556

Merged
probot-auto-merge[bot] merged 2 commits into
u-root:mainfrom
abrender:dhcp-error-checking
Nov 11, 2022
Merged

Fix error checking fbnetboot/main.go#2556
probot-auto-merge[bot] merged 2 commits into
u-root:mainfrom
abrender:dhcp-error-checking

Conversation

@abrender

Copy link
Copy Markdown
Contributor

dhcp4 and dhcp6 each iterate through the result of netboot.RequestNetbootv{4,6} before checking the error returned by the call. This may lead to situations where the code invoces the Summary() method on a nil pointer.

The called function, ReqestNetbootv4, has a code path where it may return err==nil along with invalid data. That is being fixed separately in insomniacslk/dhcp#483.

I stumbled across this while checking on #2517

Signed-off-by: Avi avibrender@gmail.com

`dhcp4` and `dhcp6` each iterate through the result of `netboot.RequestNetbootv{4,6}` before checking the error returned by the call. This may lead to situations where the code invoces the `Summary()` method on a nil pointer.

The called function, `ReqestNetbootv4`, has a code path where it may return err==nil along with invalid data.  That is being fixed separately in insomniacslk/dhcp#483.

I stumbled across this while checking on #2517

Signed-off-by: Avi <avibrender@gmail.com>
@abrender abrender mentioned this pull request Nov 11, 2022
@codecov

codecov Bot commented Nov 11, 2022

Copy link
Copy Markdown

Codecov Report

Base: 73.67% // Head: 73.67% // No change to project coverage 👍

Coverage data is based on head (79c3d70) compared to base (6e96997).
Patch has no changes to coverable lines.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2556   +/-   ##
=======================================
  Coverage   73.67%   73.67%           
=======================================
  Files         405      405           
  Lines       41282    41282           
=======================================
  Hits        30416    30416           
  Misses      10866    10866           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@abrender abrender marked this pull request as ready for review November 11, 2022 03:07
@abrender abrender requested a review from rminnich November 11, 2022 15:31
Comment thread cmds/boot/fbnetboot/main.go
@rminnich rminnich added automerge Applying this label auto-merges the PR when ready Awaiting author Waiting for new changes or feedback for author. labels Nov 11, 2022
@probot-auto-merge probot-auto-merge Bot merged commit d1a7b8e into u-root:main Nov 11, 2022
@abrender abrender deleted the dhcp-error-checking branch November 11, 2022 19:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automerge Applying this label auto-merges the PR when ready Awaiting author Waiting for new changes or feedback for author.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants