[PATCH] if_axgbe: properly release resource in error case

[ztong0001_gmail.com]

Summary

The allocated resources should be properly released on error case,
otherwise kernel will print

ax0: Invalid mac address
ax0: IFDI_ATTACH_PRE failed 22
device_attach: ax0 attach returned 22
pci0:1:0:0: Device leaked memory resources

[ztong0001_gmail.com]

Dear Reviewers,
I was doing some code review and found some issues in the NIC driver.
I am a completely newbie and I'd appreciate it if someone could give some comments on the patch.
Thanks and have a good one.

• Tong

[markj]

The patch is incomplete: we should also destroy the taskqueue created near the end of the function, and axgbe_alloc_channels() also performs some memory allocations that are not freed.

sys/dev/axgbe/if_axgbe_pci.c

428

No need for parentheses around error numbers, same below.

559

Per style(9) this should be return (ret);.

[ztong0001_gmail.com]

Hi Mark,
Thanks for your review.
I have updated the patch to address the issues.
Thanks again.

• Tong

[markj]

When uploading raw diffs, please be sure to include context (-U9999999) per https://wiki.freebsd.org/Phabricator#Create_a_Revision_via_Web_Interface .

sys/dev/axgbe/if_axgbe_pci.c

564

These comments don't say anything useful, let's please remove them.

778

Why not pass a struct axgbe_if_softc * directly?

782

There should be a newline after local variable declarations.

789

This code is copied from axgbe_if_queues_free(), so let's also modify axgbe_if_queue_free() to call this new function instead of duplicating code.

[ztong0001_gmail.com]

Thanks for the comments!
I have revised the patch accordingly.
Thanks!

• Tong

[ztong0001_gmail.com]

looks like the patch got changed somehow by the site, upload again

[markj]

Looks ok to me, just a couple of style bugs. I presume you successfully tested the latest version of the patch?

sys/dev/axgbe/if_axgbe_pci.c

80

774

[ztong0001_gmail.com]

Thank you, Mark!
I have tested on my local setup and it works fine.
Thanks and have a nice weekend!