From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752971AbbCPHq1 (ORCPT ); Mon, 16 Mar 2015 03:46:27 -0400 Received: from cantor2.suse.de ([195.135.220.15]:48303 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751435AbbCPHqY (ORCPT ); Mon, 16 Mar 2015 03:46:24 -0400 Date: Mon, 16 Mar 2015 08:46:20 +0100 From: Jan Kara To: Fabian Frederick Cc: Jan Kara , linux-kernel@vger.kernel.org Subject: Re: [PATCH 4/5 linux-next] udf: rename udf_get_filename() Message-ID: <20150316074619.GB4934@quack.suse.cz> References: <1426020276-13609-1-git-send-email-fabf@skynet.be> <1426020276-13609-4-git-send-email-fabf@skynet.be> <20150314065222.GD2888@quack.suse.cz> <886970217.268830.1426408475073.open-xchange@webmail.nmp.proximus.be> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <886970217.268830.1426408475073.open-xchange@webmail.nmp.proximus.be> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Fabian, On Sun 15-03-15 09:34:35, Fabian Frederick wrote: > > On 14 March 2015 at 07:52 Jan Kara wrote: > > > > > > On Tue 10-03-15 21:44:34, Fabian Frederick wrote: > > > udf_readdir(), udf_find_entry() and udf_pc_to_char() use > > > udf_get_filename to obtain name length. Give that function > > > an appropriate name. > >   Hum, have you read what that function does? It actually converts the name > > to a different format and returns converted length. So your name is IMHO > > more confusing - it's as if sprintf() was called sprintf_length()... Not > > applied. > > Ok for the name but AFAICS there's still a problem with error management in > udf_get_filename(). > We return 0 when not able to allocate filename and callsites don't seem to > relate the real problem. Umm, we have three callsites: udf_readdir() - that skips the name if returned length is 0. Arguably we should return error to userspace if we didn't emit any name yet. But then all other error handling in that function should behave like that. udf_find_entry() - again we skip name if returned length is 0. Again we could do better in error handling but that would require rewriting udf_find_entry() to propagate errors up the stack instead of just returning NULL. udf_pc_to_char() - This forgets to check and can return error. I'm happy to take a fix for that (or I will write it unless you do). So returning error value < 0 for error from udf_get_filename() would look OK to me. But given how udf_readdir() and udf_find_entry() behave it won't help too much. But it's a step in the right direction. Thanks for spotting this. Honza -- Jan Kara SUSE Labs, CR