From mboxrd@z Thu Jan 1 00:00:00 1970 From: laijs@cn.fujitsu.com (Lai Jiangshan) Date: Sat, 17 Jan 2009 10:57:39 +0800 Subject: [ltt-dev] [PATCH 11/13] implement ltt-ascii In-Reply-To: <20090116031359.GB9885@Krystal> References: <496C5A56.7060203@cn.fujitsu.com> <20090115200214.GB2462@Krystal> <496FE2ED.5070405@cn.fujitsu.com> <20090116031359.GB9885@Krystal> Message-ID: <49714923.2040106@cn.fujitsu.com> Mathieu Desnoyers wrote: >>>> +struct dentry *ltt_txt_create(struct ltt_trace_struct *trace, >>>> + struct ltt_channel_struct *ltt_channel) >>>> +{ >>>> + struct dentry *entry = debugfs_create_file(ltt_channel->channel_name, >>>> + S_IRUSR | S_IRGRP, trace->dentry.txt_root, ltt_channel, >>>> + <t_txt_fops); >>>> + >>> We should handle the error case here. And make sure the ltt_txt_remove >>> won't forget to put the channel on error. >>> >> when the error case here, we return NULL, and nothing done. >> What we need to handle? >> > > debugfs_create_file() can return -ENODEV if not enabled in the kernel. > Clearly this should never happen because our build depends on it. But > still, I'd be more comfortable dealing with the IS_ERR() case (if new > return values are added to debugfs in the future especially). > I remember that LTT selects DEBUG_FS, so it's OK. Most code use one of the styles: 0 -- fail other -- success -Eerrno -- fail other -- success but debugfs_create_file() is mussed up. Lai.