[PATCH] drm/i915: Allow NULL memory region

[Tvrtko Ursulin]

On 25/07/2024 16:58, Dan Carpenter wrote:
> On Thu, Jul 25, 2024 at 08:48:35AM +0100, Tvrtko Ursulin wrote:
>>
>> Hi,
>>
>> On 12/07/2024 22:41, Jonathan Cavitt wrote:
>>> Prevent a NULL pointer access in intel_memory_regions_hw_probe.
>>
>> For future reference please include some impact assessment in patches tagged
>> as fixes. Makes maintainers job, and even anyone's who tries to backport
>> stuff to stable at some future date, much easier if it is known how
>> important is the fix and in what circumstances can the problem it is fixing
>> trigger.
>>
> 
> As someone doing backport work, I think this patch is fine.  Everyone
> knows the impact of a NULL dereference in probe().
> 
> I guess with patches that add NULL dereferences, the trick is
> understanding when people are adding NULL checks to make a static
> checker happy or when it's a real bug.  But the fault lies with the
> people adding NULL checks just to make the tools happy.  Some of these
> pointless NULL checks end up in stable, but it's fine, extra NULL checks
> never hurt anyone.  If the maintainer wants to be extra safe by adding
> NULL checks then who are we to say otherwise.
> 
> In other words normal patches shouldn't have to say. "I'm not lying" at
> the end.  It should be the pointless patches which say, "I'm doing a
> pointless thing.  Don't bother backporting."
> 
> Most stable patch backports are done automatically and people have
> various tools and scripts to do that.  If the tools don't handle this
> patch automatically then they are defective.

Right, and every few releases maintainers and authors get a bunch of 
emails for patches which did not apply to some stable tree.

In which case someone has to do manual work and then it is good to know 
how important it is to backport something. For cases when it is not 
trivial. It does not apply to this patch, but as a _best practice_ it is 
good if the commit message explains the impacted platforms and scenarios.

In this case I can follow the Fixes: tag and see the fix that this 
patches fixes is only about ATS-M. Which if it was a more complicated 
patch might be a reason to not need bother backporting past some kernel 
version where platform X wasn't even supported.

Therefore I think my point is that best practice is to include this the 
commit text, so any future maintainer/backporter does not have to re-do 
detective work, stands.

Regards,

Tvrtko