Fix for #396 #408
Fix for #396 #408
Conversation
|
In daemons/gptp/linux/src/linux_hal_i210.cpp pci_connect does not call igb_attach_tx, should it ? |
|
We have a large "automotive" pull request pending that I would like to merge before addressing this one. |
| continue; | ||
| } | ||
| /*igb_attach_tx missing here ???*/ |
There was a problem hiding this comment.
I'm not positive, but I think agb_attach_tx is only needed when dealing with the userspace interface to transmit on the AVB high-priority queues. I don't think the gPTP implementation uses that interface.
|
Just fixed errno to err. |
| continue; | ||
| } | ||
| err = igb_attach_tx(igb_dev); | ||
| if (err) { | ||
| printf("attach_tx failed! (%s)\n", strerror(err)); /*errno or err ??*/ | ||
| printf("attach_tx failed! (%s)\n", strerror(err)); |
There was a problem hiding this comment.
I like these changes, but I'm left with the question about what it means to call strerror() with an error returned from a user library. Seems to me there should be a igb_strerror() function since the igb library is the entity that understands what it's returned error codes actually mean.
There was a problem hiding this comment.
The igb library returns system error codes. I don't see anything wrong with this approach. If we want to create our own set of user codes that's fine, but, in my opinion unnecessary. I think an intermediate approach might be to just wrap strerror in igb_strerror() and in the future it could be changed a little more easily.
|
My vote is to accept this pull request as-is and create an issue to review what calling strerror(err) on igb error returns should really be. |
|
i agree. |
|
I don't see anything wrong with strerror being called on an error value returned by a user library if the library has been designed to use standard error numbers. It is a bit limiting in the sort of errors you can return, but for things that mostly fail in standard ways, it's nice to not have to redefine a whole alternate set of error codes. Anyway, looks like consensus is to merge. |
: Merge pull request Avnu#408 from RikusW/fix-#396)
… from RikusW/fix-#396
Fix for #396
pci_connect seems to be duplicated 5 times...