> On 16 March 2015 at 08:46 Jan Kara <j...@suse.cz> wrote: > > > Hi Fabian, > > On Sun 15-03-15 09:34:35, Fabian Frederick wrote: > > > On 14 March 2015 at 07:52 Jan Kara <j...@suse.cz> 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.
Thanks a lot Jan. I hope the patch I just sent is ok. Regards, Fabian > > Honza > > -- > Jan Kara <j...@suse.cz> > SUSE Labs, CR -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
