From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f51.google.com (mail-wr1-f51.google.com [209.85.221.51]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 011693438AC for ; Wed, 3 Jun 2026 17:34:21 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.221.51 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1780508064; cv=none; b=jbjWK9NURrUzHCCJEQNnBuLcUikCZF0OV8ERBbPM7ywGLvHLXHipuWm6PX9OAbB9uCFozjz6+u16Hj93hnJ8G/1wjbwHaakeofoWi/INTCBYwjDLNK42mmp0o3d65uCyWKhs5v/r8Gr9d4tAAtI77jT0ubuZ1nlTewtyUikEGkk= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1780508064; c=relaxed/simple; bh=LUlX0sVoiHxqZLUHXdeDjTvkYDAJRW4V6r2+fFCsHhY=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=NLwJtNt/VXnmXAPwmPurZda+827liKmIk8KBsmey+/ua8yZ3Z3W6Fk0hhwcvZJbENEXKB4bD4WW8UXneUAHtcrC7inb+vgU8hnmpDzRla3QnHoAh8JMe5QZvbp5WFnVjEWha9jykJzj3+nvLguTplLz0gYlKHh20TTv0hh+HvxE= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=philpotter.co.uk; spf=pass smtp.mailfrom=philpotter.co.uk; dkim=pass (2048-bit key) header.d=philpotter-co-uk.20251104.gappssmtp.com header.i=@philpotter-co-uk.20251104.gappssmtp.com header.b=dAjCUT9C; arc=none smtp.client-ip=209.85.221.51 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=philpotter.co.uk Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=philpotter.co.uk Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=philpotter-co-uk.20251104.gappssmtp.com header.i=@philpotter-co-uk.20251104.gappssmtp.com header.b="dAjCUT9C" Received: by mail-wr1-f51.google.com with SMTP id ffacd0b85a97d-45fd464d51fso2372786f8f.3 for ; Wed, 03 Jun 2026 10:34:21 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=philpotter-co-uk.20251104.gappssmtp.com; s=20251104; t=1780508060; x=1781112860; darn=vger.kernel.org; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date:from:to :cc:subject:date:message-id:reply-to; bh=1iTr6BJdTO1dRuaVLoXzUA4PzL2cQFTNvy8pvk9M2Lc=; b=dAjCUT9CVR4ekmYtiCjmJNwHNN1fIHg9SbR9t/Dr6pQt9nWCHuCgPmIUSnZF+8bZRb OYPZIiqjPcBNAtqPxF+O5vsla4PeTYuOCBOhL62kxZQ9HmpGOJ8rlZZ3UF1zg71LrXCn 80YTeev27QHzsI2C36tJq8e0jIrrlUAcgsdT2zbRUmm/zNHv6tO+8xDeNKa4Aknbxga8 4ogl9E3V5/KwT39zqHfQtQDjog+VPVEZRxeEJGNCAIxvzMo6/5Ig7ovEVMnaqH8JIH70 HhnJQf0ZU0tWzpIEfMfBH8wDSuVtSX4JjcRJ1k3OBhY6GVbFEHo1KLsHX3O0kjyyVkhK sHsQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1780508060; x=1781112860; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=1iTr6BJdTO1dRuaVLoXzUA4PzL2cQFTNvy8pvk9M2Lc=; b=HBCHXUon4qSdCZZBoJrAw/j+TqjW2O5mO7qYyb4yoGlvpEqT9Lvq8iauVzflhGiOs1 9sn+c6dEyDUowhmFdH7q+UmmmIYP5G4JrVx7Va74TKI9iqkvolkXW7gzDUS9itlWB0r7 +p/gj8gsd5vhB80uXtkINKsZqQPGJoFNVP+jlspaYntN6S9H99Nr3bAkyO09TBwWTqwV xYf93wFCUsDrlJspBem9K4/+GDHQJY2RIOypcWuiojwPg1nWWErbffmiUroK5ywRBBzS YTIg+40q2A5iC/w+JKEWquEDvkixJuiIwyUcabGsGBhPBZG+FQU4h8QcoaMSQmO3QDEQ AgLQ== X-Gm-Message-State: AOJu0Yzq3SqgmydKp09azVsjDEZrimnJq3Ds85hIoSqwwm8ZoSK6NOyE yCtAFfMnRCJ7WhED7lpBxOXansI7Q6WwNssGcYQtNKK/cqjuin/UvA8tpaX0g5DwM0Q= X-Gm-Gg: Acq92OHAPLISX6Pywi2AMtOC2NTNlZGPvhZ7Dl9kTMP+ioywu5f89whHsySfJfG0GAl igmEVpgRpCaVP6BnuKfKfHhV/WGLAZ5Ur8iqlt4aqt9suGUnX4hmUMvdgffjiBwems7orxFo4ye fOCRrhRFvtNIGF8kG/2phyayOW5nefIz5VPJFe0WJHGo99Ta+ChtLSM6haQXg3BrYzybczmPA3V qGney19JR3bho7GsRBIhW4Cf9vvbbz4933xfL2JA+ZRd71m4VMhUxvEiAjLuu+oS7psPSikHnld 1zrH8t3gwQIchDJQffslL2ZMWkbwBr2/jtkz3IraIA6PRXa5heVUe9Otri+VnXOSd6ujoLgmWI6 wyDv+KvsWBGUoBeqSs0bNMjzr/lJwciGJNNJHJAMOsZStJc3coM3aOURgaMGKj7a6V4j+c6Akg+ eD++igCJjSO9Y6UEqCXovQCKLW0EaUkLBzVyK85SPz48aw8RqSa/ehG1weUAiF1q/+WGyRyJXQn ersABlH9mLSK2BXuaBS X-Received: by 2002:a5d:64e4:0:b0:460:1168:6dd4 with SMTP id ffacd0b85a97d-460217bdbe3mr6825749f8f.14.1780508060253; Wed, 03 Jun 2026 10:34:20 -0700 (PDT) Received: from equinox (2.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.6.1.f.d.0.b.8.0.1.0.0.2.ip6.arpa. [2001:8b0:df16::2]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-4601f35eae5sm9141624f8f.33.2026.06.03.10.34.19 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 03 Jun 2026 10:34:19 -0700 (PDT) Date: Wed, 3 Jun 2026 18:34:18 +0100 From: Phillip Potter To: Jedrzej-Sz Cc: linux-kernel@vger.kernel.org, phil@philpotter.co.uk Subject: Re: [PATCH] cdrom: modernize locking, memory allocation, and fix checkpatch warnings Message-ID: References: <20260602150718.545925-1-jedrzej.szoszo@gmail.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20260602150718.545925-1-jedrzej.szoszo@gmail.com> On Tue, Jun 02, 2026 at 05:07:18PM +0200, Jedrzej-Sz wrote: > From: Jędrzej Szoszorek > > This patch introduces several modernizations and fixes to the legacy cdrom driver to align it with current kernel standards: > > Concurrency: Added cdrom_events_lock spinlock to protect vfs_events and ioctl_events from data races. Wrapped use_count modifications in cdrom_open() and cdrom_release() with cdrom_mutex. > > Memory Safety: Replaced obsolete and unsafe allocation methods with kzalloc() and struct_size() to prevent potential buffer overflows when allocating flexible arrays (e.g., struct cdrom_changer_info). > > Sysctl API: Refactored cdrom_sysctl_info() to use dynamic memory allocation for the info buffer instead of a hardcoded string array, preventing truncation with multiple drives. Additionally, removed the empty {} sentinel at the end of cdrom_table to fix runtime warnings with the modern register_sysctl API. > > Error Handling: Replaced inappropriate -ENOSYS returns in ioctl paths with -ENOTTY, as expected by current kernel design patterns. > > Includes: Migrated from to to resolve compilation errors on modern mainline trees, updating unaligned memory access macros accordingly. > > Code Style: Fixed numerous formatting issues, block comment alignments, trailing whitespaces, and replaced hardcoded function names in debug logs with __func__ to ensure checkpatch.pl compliance. > > Signed-off-by: Jędrzej Szoszorek > --- > Note: During the review and refactoring of this legacy driver, I used > an AI assistant (Gemini) to help identify outdated API patterns (like > missing event locks, legacy allocations, and sysctl sentinels). All > suggestions were manually audited, verified against current kernel APIs, > and successfully tested in a minimal QEMU environment. > Dear Jędrzej, Thank you for the patch. In its current form however, I don't feel like it's something I can effectively review. Few comments from me: (1) This is a huge patch. 700+ insertions and 600+ removals. If you're serious about the changes, please split them up. (2) I'm not convinced of the value of the changes, for example just reformatting the contributors list. It doesn't add much value as far as I can see, whilst of course being structurally correct. It is 'churn' technically speaking. (3) If you are changing error values and other such things, I'm curious to know how these changes have been tested, and if they introduce regressions in calling code? When this is such a big refactor, "successfully tested in a minimal QEMU environment." doesn't cut it for me at present. Given the legacy/widespread nature of the driver, whilst I'm not explicitly anti-AI, I would hope that all these changes are well understood. In particular, that they solve real problems/issues without introducing regressions. If that's the case, then I'm happy to take a look at a slimmed down and revised set of patches that effect equivalent changes to the driver. All the best, Phil