Received-SPF: pass (google.com: domain of linux-kernel+bounces-189519-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
From: Jiasheng Jiang <jiashengjiangcool@outlook.com>
To: viro@zeniv.linux.org.uk
Cc: brauner@kernel.org,
	jack@suse.cz,
	arnd@arndb.de,
	gregkh@suse.de,
	linux-fsdevel@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	Jiasheng Jiang <jiashengjiangcool@outlook.com>
Subject: Re: [PATCH] libfs: fix implicitly cast in simple_attr_write_xsigned()
Date: Sat, 25 May 2024 19:55:52 +0000
Message-ID:
 <BL0PR03MB41610A9302ADA6A5022A306BADF62@BL0PR03MB4161.namprd03.prod.outlook.com>
Content-Transfer-Encoding: 8bit
Content-Type: text/plain
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-MessageSentRepresentingType: 1
X-MS-PublicTrafficType: Email
X-MS-TrafficTypeDiagnostic: BL0PR03MB4161:EE_|MN2PR03MB5295:EE_
X-MS-Office365-Filtering-Correlation-Id: 94f7d27e-fffe-4824-31ed-08dc7cf4b2fc
X-Microsoft-Antispam:
	BCL:0;ARA:14566002|461199019|4295299012|3420499023|3430499023|440099019|3412199016|1710799017;
X-Microsoft-Antispam-Message-Info:
	tY0ucaV94BG6iio5V/Hc6VqoU20f4lJdEekWRdvy8XdA+lsrlKP2t0+Q27vPQl6MaxLJDHpp/pENhYI+ywINzIiksvgAtA+OrWPQHbhad7gsMEdU84IlmxHjndq3bNP4/QSSOZpiInCYyEqXvfnJIASpMvONbwDbep61hRqYCWopNIj091NnzTpIcKVjamahsaQFo4UOX7l5MJdZr5HF8cmbiDMajNCoe/KqnApXxlxrkRGDvROJSicbobnYkzji4yn9+dJDfs+dRiThzhQELo11/muEMvw3/z3W3f5rNiesV/EAFCzIA1XQ3g9iFUWiCP1ioqsgMZXFmv+EYLSEIQW9E+fMaOONHJplqEaQWrTQZ5Fzj+yanQRoVYDIIoBMHzroIRl/BTQReuxjQ97jeCsY+8yznJrgkGodCjlZyo7CM/u2y1Y7nj4HGndtqwTTmxWG2nwVoTltkhRzVPu11NxKorBhAxtTyGbx22OeqnYmVi8dFCcVYkufoo/UERvb6xI2VSB9/O310AqxtohC83fy+3hl5Cne0eeVxnq+FwXPXWI5L4NWUjHDF6QAWwrAEE5hAhTeZARHuLQDVvv7TdWcyKkvEoit5H8Uz9NOnAQsbC6VNQDa3WWNecI8rJcT
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?us-ascii?Q?4Vy9FI1VGFkEJewJA4aRy/ifcirjw+QxM557Z5Fli2g1rTY59tYn83g/JJie?=
 =?us-ascii?Q?SvGq9yLbynv7n7ZGPo3JOcVc0mLAIrNDS3Kcp6SX0Ad85wz0zm9RuNDtxY/Q?=
 =?us-ascii?Q?IQll+UyYAUqZKHARYvNWkewZIVB1U4grWfpQD/hYzqBUn+ExS2SfTaM7KEel?=
 =?us-ascii?Q?dHanXklum1ewQ5041VRDfWFcDez8BB/KTIzM90yDajZVH8VC3UrmcnKRL1+1?=
 =?us-ascii?Q?8CVnadv33M50YXFdeO986hGTTTip+0xTWt/S8ZDTcsvOK3WRPldXYGG//Byk?=
 =?us-ascii?Q?uY5hftnd4KXgvUfxuD8DOBUHDv2I1pTucJTkYs+FtGzf30Zh9Wa55qbeMAgK?=
 =?us-ascii?Q?PhxBulyUqYZl53O4milT3vsP+AaEmk3GauHLWUkvnVE2qb4Ijkzorh1hV1Kx?=
 =?us-ascii?Q?KRP0MMp//9i+7TdwwZa6KJwooQFgbfpNKkY30zTrg8OTQ6El8zI0RuMIgbpU?=
 =?us-ascii?Q?20yoX4g9ntZv4tQvC4+0dhjrVjFM8RXr6Py6sEmtFvaeSFYns2t/1fVMyTHS?=
 =?us-ascii?Q?+Bu6xpvAkVFDcHBqf8tY72YQso0PqfiBmUrJ3qw2/3hCFuhGIrO4DDfX6KHk?=
 =?us-ascii?Q?lkqrUUtkrnM0hzdm4z74958WnWVZpX4RI7az2gRZYmborQIGBOkYulQJyQxp?=
 =?us-ascii?Q?bl3P6dXtNS0YjpBrfq9Y3s/s/Lw5i1sFmPgxxpyV/SdzXTbBlyKXLiz5cyLs?=
 =?us-ascii?Q?vyX9k/woYv0TObuyIMGQkkl52R4S9JR0jRrdINJQbixZYjsHM8V5NfzVx4iR?=
 =?us-ascii?Q?emPPhxZiLsF7cfPp1sd/l1/QEnMatxvBa1uwPWSi0eSWymotPbRE4mP1ufLp?=
 =?us-ascii?Q?9ZGnG80Wa0BKH5g3dFQULIQ2SzIHdVcgAEYPXRI4OLFW5lqBwlCIcPfvehNc?=
 =?us-ascii?Q?uy1hRZWXWhfc0rgge5LrZ39WY5DjuCzYdB2yJPt7TfbVBICz+6c8wjAYHCmo?=
 =?us-ascii?Q?oO96RsA17WXRG8fEfJ6lGoIue0TpSk95ou9zQ5YEHXo03mqcRfyjYPFmiIEv?=
 =?us-ascii?Q?FnrJWjn91UgbUcQYSdOPxxMk+0ZWvC+OIsg5mCKvkcB+fRYbx6saYC8dk2cb?=
 =?us-ascii?Q?WY3yCCwZQwEckw9G9/+fklllYZtsb0mEIqQCdSr4mkSfqi7St8lLO3PElB8X?=
 =?us-ascii?Q?WCwnqIOKeGs7bYzBlGbdIbIuMSLEsEeDF4ponu9jDNKusBYAmdrtZo+x/jMb?=
 =?us-ascii?Q?/PzQTP+v4YLnoiJMfdRGCh2gjyuf2DVjyMYkay23Z7OVlN8EfEK1ihKg/2P1?=
 =?us-ascii?Q?Mn2Kumr+O7NlktjhW8qS?=
X-OriginatorOrg: outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 94f7d27e-fffe-4824-31ed-08dc7cf4b2fc
X-MS-Exchange-CrossTenant-AuthSource: BL0PR03MB4161.namprd03.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 25 May 2024 19:56:04.8225
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 84df9e7f-e9f6-40af-b435-aaaaaaaaaaaa
X-MS-Exchange-CrossTenant-RMS-PersistedConsumerOrg: 00000000-0000-0000-0000-000000000000
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN2PR03MB5295

> On Wed, May 15, 2024 at 03:17:25PM +0000, Jiasheng Jiang wrote:
>> Return 0 to indicate failure and return "len" to indicate success.
>> It was hard to distinguish success or failure if "len" equals the error
>> code after the implicit cast.
>> Moreover, eliminating implicit cast is a better practice.
> 
> According to whom?
> 

Programmers can easily overlook implicit casts, leading to unknown
behavior (e.g., this bug).
Converting implicit casts to explicit casts can help prevent future
errors.

> Merits of your ex cathedra claims aside, you do realize that functions
> have calling conventions because they are, well, called, right?
> And changing the value returned in such and such case should be
> accompanied with the corresponding change in the _callers_.
> 
> Al, wondering if somebody had decided to play with LLM...

As the comment shows that "ret = len; /* on success, claim we got the
whole input */", the return value should be checked to determine whether
it equals "len".

Moreover, if "len" is 0, the previous copy_from_user() will fail and
return an error.
Therefore, 0 is an illegal value for "len". Besides, in the linux kernel,
all the callers of simple_attr_write_xsigned() return the return value of
simple_attr_write_xsigned().

-Jiasheng