Re: [PATCH 1/3] media: i2c: gc0310: fix probe error handling and unwind resources properly

[Sanjay Chitroda]

On 2 April 2026 12:36:33 am IST, Hans de Goede <hansg@xxxxxxxxxx> wrote:
>Hi,
>
>On 1-Apr-26 20:16, Sanjay Chitroda wrote:
>> From: Sanjay Chitroda <sanjayembeddedse@xxxxxxxxx>
>> 
>> The GC0310 probe path currently performs error cleanup by jumping to a
>> common label that mirrors the driver's remove() callback. This is unsafe,
>> as remove() assumes that the subdevice has been fully registered with
>> the V4L2 framework, media and control resources have been initialized.
>
>That is simply not true, all functions called in remove() internally
>check if their init counter-part has succeeded and if not are a no-op.
>
>If you're aware of any specific calls in remove() where this is not
>the case, please explicitly describe these cases and describe an
>example exit-error path from probe() where things actually go wrong.
>

Hi Hans,

Thanks for the clarification - agreed, the remove helpers are defensively implemented and the existing code is not incorrect for a functional point. I should not have stated gc0310_remov() from a probe failure is unsafe.

>> Calling remove() from probe can result in unregistering or cleaning up
>> subdevice, leading to resource leaks and subtle lifecycle bugs.
>> 
>> Rewrite the probe() error handling to unwind resources explicitly, using
>> fine‑grained goto labels along with appropriate error logs. Each failure
>> path now frees only successfully acquired resource, without remove().
>> 
>> This aligns the driver with standard V4L2 sensor lifecycle expectations
>> and avoids incorrect teardown during probe failures.
>
>The rest of this reads very much like this was AI generated.
>
>Did you use AI to generate these patches ? If so please:
>
>Make sure you actually understand what the patch is doing and
>very yourself that it actually is correct, which in this case
>I believe it is not.
>
>Regards,
>
>Hans
>

Yes, I did use AI assistance to help draft the commit message, but the patch logic itself was written and reviewed by me. However, your feedback makes it clear that i did not sufficiently validate internals of existing remove() based cleanup.


I would like to propose commit message that align with change and existing kernel internals:

-------
media: i2c: gc0310: make probe error unwinding explicit

The gc0310 probe path unwinds failures by jumping to a single label remove-style cleanup.

Refactor the probe error handling so that resources are unwound explicitly and in reverse order of initialization using fine-grained goto labels.

This improves clarity and maintains symmetry with the probe initialization path.

No functional change intended.
-------

Kindly share your input on the same, according I will plan to resend v2 with an updated commit message as above.

>