From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: dma-buf: Add the SGT DMA mapping type Date: Wed, 18 Feb 2026 11:37:56 +1000 Message-ID: In-Reply-To: <2-v1-b5cab63049c0+191af-dmabuf_map_type_jgg@nvidia.com> References: <0-v1-b5cab63049c0+191af-dmabuf_map_type_jgg@nvidia.com> <2-v1-b5cab63049c0+191af-dmabuf_map_type_jgg@nvidia.com> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit MIME-Version: 1.0 Patch Review > +enum dma_sgt_requires_p2p { > + DMA_SGT_NO_P2P = 0, > + DMA_SGT_EXPORTER_REQUIRES_P2P_DISTANCE, > + DMA_SGT_IMPORTER_ACCEPTS_P2P, > +}; A single enum serves double duty for `exporter_requires_p2p` and `importer_accepts_p2p`. The naming of the enum values tries to prevent misuse, but the type system doesn't enforce it. Nothing stops setting `exporter_requires_p2p = DMA_SGT_IMPORTER_ACCEPTS_P2P`, which would silently fall through the switch to return 0 (match succeeds). Would separate enums or booleans be clearer? > + switch (exp->sgt_data.exporter_requires_p2p) { > + case DMA_SGT_NO_P2P: > + return 0; > + case DMA_SGT_EXPORTER_REQUIRES_P2P_DISTANCE: > + ... > + } > + return 0; The default fall-through silently accepts any value. A `WARN_ON_ONCE` or explicit default handling would catch programming errors. > + u8 importer_accepts_p2p; > + u8 exporter_requires_p2p; These are `u8` rather than `enum dma_sgt_requires_p2p`. Intentional for size, or an oversight? --- Generated by Claude Code Patch Reviewer