Re: [PATCH] perf: Handle DT_UNKNOWN on filesystems that don't support d_type

[Arnaldo Carvalho de Melo <acme@ghostprotocols.net>]

Em Sat, Nov 20, 2010 at 06:42:19PM -0600, Shawn Bohrer escreveu:
> Some filesystems like xfs and reiserfs will return DT_UNKNOWN for the
> d_type.  Handle this case by calling stat() to determine the type.

Thanks for the fix, just waiting for some more reviewers to chime in,
seems odd, like readdir_r has a bug.

Even if that is the case we'll have to cope, and doing the extra stat
only when in "doubt" (i.e. when getting DT_UNKNOWN) seems the right
thing to me.

- Arnaldo
 
> Signed-off-by: Shawn Bohrer <sbohrer@rgmadvisors.com>
> ---
>  tools/perf/builtin-trace.c |   35 +++++++++++++++++++++++++++--------
>  1 files changed, 27 insertions(+), 8 deletions(-)
> 
> diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
> index 86cfe38..9cb24fa 100644
> --- a/tools/perf/builtin-trace.c
> +++ b/tools/perf/builtin-trace.c
> @@ -301,17 +301,36 @@ static int parse_scriptname(const struct option *opt __used,
>  	return 0;
>  }
>  
> -#define for_each_lang(scripts_dir, lang_dirent, lang_next)		\
> +/* Helper function for filesystems that return a dent->d_type DT_UNKNOWN */
> +static int is_directory(const char *base_path, const struct dirent *dent)
> +{
> +	char path[PATH_MAX];
> +	struct stat st;
> +
> +	sprintf(path, "%s/%s", base_path, dent->d_name);
> +	if (stat(path, &st))
> +		return 0;
> +
> +	return S_ISDIR(st.st_mode);
> +}
> +
> +#define for_each_lang(scripts_path, scripts_dir, lang_dirent, lang_next)\
>  	while (!readdir_r(scripts_dir, &lang_dirent, &lang_next) &&	\
>  	       lang_next)						\
> -		if (lang_dirent.d_type == DT_DIR &&			\
> +		if (((lang_dirent.d_type == DT_DIR &&			\
> +		      lang_dirent.d_type != DT_UNKNOWN) ||		\
> +		     (lang_dirent.d_type == DT_UNKNOWN &&		\
> +		      is_directory(scripts_path, &lang_dirent))) &&	\
>  		    (strcmp(lang_dirent.d_name, ".")) &&		\
>  		    (strcmp(lang_dirent.d_name, "..")))
>  
> -#define for_each_script(lang_dir, script_dirent, script_next)		\
> +#define for_each_script(lang_path, lang_dir, script_dirent, script_next)\
>  	while (!readdir_r(lang_dir, &script_dirent, &script_next) &&	\
>  	       script_next)						\
> -		if (script_dirent.d_type != DT_DIR)
> +		if ((script_dirent.d_type != DT_DIR &&			\
> +		     script_dirent.d_type != DT_UNKNOWN) ||		\
> +		    (script_dirent.d_type == DT_UNKNOWN &&		\
> +		     !is_directory(lang_path, &script_dirent)))
>  
>  
>  #define RECORD_SUFFIX			"-record"
> @@ -466,14 +485,14 @@ static int list_available_scripts(const struct option *opt __used,
>  	if (!scripts_dir)
>  		return -1;
>  
> -	for_each_lang(scripts_dir, lang_dirent, lang_next) {
> +	for_each_lang(scripts_path, scripts_dir, lang_dirent, lang_next) {
>  		snprintf(lang_path, MAXPATHLEN, "%s/%s/bin", scripts_path,
>  			 lang_dirent.d_name);
>  		lang_dir = opendir(lang_path);
>  		if (!lang_dir)
>  			continue;
>  
> -		for_each_script(lang_dir, script_dirent, script_next) {
> +		for_each_script(lang_path, lang_dir, script_dirent, script_next) {
>  			script_root = strdup(script_dirent.d_name);
>  			str = ends_with(script_root, REPORT_SUFFIX);
>  			if (str) {
> @@ -514,14 +533,14 @@ static char *get_script_path(const char *script_root, const char *suffix)
>  	if (!scripts_dir)
>  		return NULL;
>  
> -	for_each_lang(scripts_dir, lang_dirent, lang_next) {
> +	for_each_lang(scripts_path, scripts_dir, lang_dirent, lang_next) {
>  		snprintf(lang_path, MAXPATHLEN, "%s/%s/bin", scripts_path,
>  			 lang_dirent.d_name);
>  		lang_dir = opendir(lang_path);
>  		if (!lang_dir)
>  			continue;
>  
> -		for_each_script(lang_dir, script_dirent, script_next) {
> +		for_each_script(lang_path, lang_dir, script_dirent, script_next) {
>  			__script_root = strdup(script_dirent.d_name);
>  			str = ends_with(__script_root, suffix);
>  			if (str) {
> -- 
> 1.7.2.3