Re: [PATCH] Fix searching of kernel_source_tree for kernel built with O option

[Hou Tao <houtao1 at huawei dot com>]

Hi, Any comments for the fix ?

On 2018/9/10 19:46, Hou Tao wrote:
> When generating kernel module for a systemtap script that uses trace-point
> probe, if the vanilla kernel is built by using O=build_path option and
> r=build_path option is passed to stap, stap will not be able to find
> kernel_source_tree and will fail on pass-2.
> 
> Linux kernel will create a symlink named source to the source tree
> for out-of-source build since (399b835be30e "kbuild: add a symlink
> to the source for separate objdirs"), so fix the problem by checking
> whether or not the symlink exists and using it as the kernel_source_tree.
> 
> Also using a new helper dir_exists() instead of file_exists() to
> ensure the existence of the directory of source tree.
> 
> Signed-off-by: Hou Tao <houtao1@huawei.com>
> ---
>  tapsets.cxx | 34 ++++++++++++++++++++++++++--------
>  util.cxx    | 13 +++++++++++++
>  util.h      |  1 +
>  3 files changed, 40 insertions(+), 8 deletions(-)
> 
> diff --git a/tapsets.cxx b/tapsets.cxx
> index ca94be4ce..8617796d8 100644
> --- a/tapsets.cxx
> +++ b/tapsets.cxx
> @@ -12391,16 +12391,34 @@ tracepoint_builder::init_dw(systemtap_session& s)
>      }
>  
>    // find kernel_source_tree from a source link, when different from build
> -  if (s.kernel_source_tree == "" && endswith(s.kernel_build_tree, "/build"))
> +  if (s.kernel_source_tree == "")
>      {
> -      string source_tree = s.kernel_build_tree;
> -      source_tree.replace(source_tree.length() - 5, 5, "source");
> -      if (file_exists(source_tree) &&
> -          resolve_path(source_tree) != resolve_path(s.kernel_build_tree))
> +      vector<string> source_trees;
> +
> +      // vendor kernel (e.g. Fedora): the source link is in the same dir
> +      // as the build tree
> +      if (endswith(s.kernel_build_tree, "/build"))
> +        {
> +          string source_tree = s.kernel_build_tree;
> +          source_tree.replace(source_tree.length() - 5, 5, "source");
> +          source_trees.push_back(source_tree);
> +        }
> +
> +      // vanilla kernel: the source link is in the build tree
> +      source_trees.push_back(s.kernel_build_tree + "/source");
> +
> +      for (unsigned i = 0; i < source_trees.size(); i++)
>          {
> -          if (s.verbose > 2)
> -            clog << _F("Located kernel source tree at '%s'", source_tree.c_str()) << endl;
> -          s.kernel_source_tree = source_tree;
> +          string source_tree = source_trees[i];
> +
> +          if (dir_exists(source_tree) &&
> +              resolve_path(source_tree) != resolve_path(s.kernel_build_tree))
> +            {
> +              if (s.verbose > 2)
> +                clog << _F("Located kernel source tree at '%s'", source_tree.c_str()) << endl;
> +              s.kernel_source_tree = source_tree;
> +              break;
> +            }
>          }
>      }
>  
> diff --git a/util.cxx b/util.cxx
> index 1b1127995..b4f1e9c25 100644
> --- a/util.cxx
> +++ b/util.cxx
> @@ -103,6 +103,19 @@ file_exists (const string &path)
>    return false;
>  }
>  
> +// Check that a dir is present
> +bool
> +dir_exists(const string &path)
> +{
> +  struct stat info;
> +
> +  if (stat(path.c_str(), &info) == 0 &&
> +      S_ISDIR(info.st_mode))
> +    return true;
> +
> +  return false;
> +}
> +
>  // Copy a file.  The copy is done via a temporary file and atomic
>  // rename.
>  bool
> diff --git a/util.h b/util.h
> index b4c4e40b7..4e30d4237 100644
> --- a/util.h
> +++ b/util.h
> @@ -72,6 +72,7 @@ const char *get_home_directory(void);
>  size_t get_file_size(const std::string &path);
>  size_t get_file_size(int fd);
>  bool file_exists (const std::string &path);
> +bool dir_exists(const std::string &path);
>  bool copy_file(const std::string& src, const std::string& dest,
>                 bool verbose=false);
>  int create_dir(const char *dir, int mode = 0777);
>