Claude review: drm/client: Do not destroy NULL modes

[Claude Code Review Bot <claude-review@example.com>]

Patch Review

**Bug analysis:** Correct. Looking at lines 856–862 of `drm_client_modeset.c`:

```c
crtcs = kzalloc_objs(*crtcs, connector_count);
modes = kzalloc_objs(*modes, connector_count);
offsets = kzalloc_objs(*offsets, connector_count);
enabled = kzalloc_objs(bool, connector_count);
if (!crtcs || !modes || !enabled || !offsets) {
    ret = -ENOMEM;
    goto out;
}
```

If `crtcs` succeeds but `modes` fails, we jump to `out:` where `modes_destroy()` is called with `modes == NULL`. Inside `modes_destroy()`:

```c
static void modes_destroy(struct drm_device *dev,
                          const struct drm_display_mode *modes[],
                          int count)
{
    int i;

    for (i = 0; i < count; i++)
        mode_replace(dev, &modes[i], NULL);
}
```

The `modes[i]` dereference with `modes == NULL` is indeed a NULL pointer dereference. The fix is correct.

**Minor suggestion:** `kfree(NULL)` is safe in the kernel, and `kfree(modes)` already follows the `modes_destroy()` call. The guard added is:

```c
if (modes)
    modes_destroy(dev, modes, connector_count);
```

This is fine, but an alternative would be to add the NULL check inside `modes_destroy()` itself, which would make it defensive at the source rather than requiring every caller to remember the guard. That said, there is currently only one caller of `modes_destroy()`, so this approach is acceptable.

**Nit on the commit message:** The commit message says `modes` "may fail to kcalloc", but the actual allocation macro used is `kzalloc_objs` (which wraps `kzalloc`), not `kcalloc`. This is a very minor inaccuracy — consider saying "may fail to allocate" instead.

**Verdict:** The fix is correct and the Fixes tag references the right commit. The patch is ready to merge with at most a commit message wording tweak.

**Reviewed-by worthy:** Yes, with the minor commit message nit noted above.

---
Generated by Claude Code Patch Reviewer