Details
Diff Detail
- Repository
- rG FreeBSD src repository
- Lint
Lint Skipped - Unit
Tests Skipped - Build Status
Buildable 64980 Build 61863: arc lint + arc unit
Event Timeline
Motivation? Why do we need gpio_pin_release to return errors?
This creates the problem of freeing the gpio structure in case of an error.
IMHO, I think all errors in gpiobus_release_pin() and gpiobus_acquire_pin() should be converted to panic().
Also passing NULL to gpio_pin_release() should be panic().
Motivation? Why do we need gpio_pin_release to return errors?
It was to have it in line with gpio_pin_acquire. I found it a little weird that only one of them would report errors.
This creates the problem of freeing the gpio structure in case of an error.
At first I was freeing on both failure and success, but I wasn't sure that was the right thing to do..
IMHO, I think all errors in gpiobus_release_pin() and gpiobus_acquire_pin() should be converted to panic().
Also passing NULL to gpio_pin_release() should be panic().
I agree for gpiobus_release_pin, since they're all programming errors. For gpiobus_acquire_pin, I think attempting to acquire a pin twice should just return an error to the caller, it's possible the caller can just recover.
All *release* functions are usually very difficult to recover, especially if they are called throw multiple layers.
This creates the problem of freeing the gpio structure in case of an error.
At first I was freeing on both failure and success, but I wasn't sure that was the right thing to do..
This leading to a situation where the malloc cannot be released in any way. If gpiobus_release_pin() fails once, then it will always fail. But this is irrelevant if we replace errors with panic...
IMHO, I think all errors in gpiobus_release_pin() and gpiobus_acquire_pin() should be converted to panic().
Also passing NULL to gpio_pin_release() should be panic().I agree for gpiobus_release_pin, since they're all programming errors. For gpiobus_acquire_pin, I think attempting to acquire a pin twice should just return an error to the caller, it's possible the caller can just recover.
Yep, exactly. I missed the multiple acquisition case originally. Only error value should be better than -1.
I went for a KASSERT instead of a panic since it's what the other gpio_* functions do. On a non-INVARIANTS kernel, if gpio is actually NULL, it will panic when we try to dereference it anyways.