Merged with modifications, see below.
Thanks!
Jérémie
On Thu, Sep 3, 2015 at 5:52 PM, Jonathan Rajotte
<jonathan.rajotte-jul...@efficios.com> wrote:
> Signed-off-by: Jonathan Rajotte <jonathan.rajotte-jul...@efficios.com>
> ---
> src/bin/lttng-crash/lttng-crash.c | 65
> +++++++++++++++++++++++++++++----------
> 1 file changed, 48 insertions(+), 17 deletions(-)
>
> diff --git a/src/bin/lttng-crash/lttng-crash.c
> b/src/bin/lttng-crash/lttng-crash.c
> index 8e57034..00a97df 100644
> --- a/src/bin/lttng-crash/lttng-crash.c
> +++ b/src/bin/lttng-crash/lttng-crash.c
> @@ -1053,35 +1053,62 @@ end:
> }
>
> static
> -int delete_trace(const char *trace_path)
> +int delete_recursive_dir(const char *path)
Renamed to "delete_dir_recursive".
> {
> - DIR *trace_dir;
> - int trace_dir_fd, ret = 0, closeret;
> + DIR *dir;
> + int dir_fd, ret = 0, closeret;
> struct dirent *entry;
>
> /* Open trace directory */
> - trace_dir = opendir(trace_path);
> - if (!trace_dir) {
> - PERROR("Cannot open '%s' path", trace_path);
> + dir = opendir(path);
> + if (!dir) {
> + PERROR("Cannot open '%s' path", path);
> return -1;
> }
> - trace_dir_fd = dirfd(trace_dir);
> - if (trace_dir_fd < 0) {
> + dir_fd = dirfd(dir);
> + if (dir_fd < 0) {
> PERROR("dirfd");
> return -1;
> }
Not introduced by your patch, but this will leak dir. I have fixed the
problem and modified your patch so that it applies cleanly on top of
it.
>
> - while ((entry = readdir(trace_dir))) {
> + while ((entry = readdir(dir))) {
> if (!strcmp(entry->d_name, ".")
> || !strcmp(entry->d_name, "..")) {
> continue;
> }
> switch (entry->d_type) {
> case DT_DIR:
> - unlinkat(trace_dir_fd, entry->d_name, AT_REMOVEDIR);
> + {
> + char subpath[PATH_MAX];
>
^ I changed this for dynamic allocation since "PATH_MAX" can become
quite large on some system configurations and we may "recurse" rather
deeply in a trace hierarchy.
> + strncpy(subpath, path,
> + sizeof(subpath));
> + subpath[sizeof(subpath) - 1] = '\0';
> + strncat(subpath, "/",
> + sizeof(subpath) - strlen(subpath) - 1);
> + strncat(subpath, entry->d_name,
> + sizeof(subpath) - strlen(subpath) - 1);
> +
> + ret = delete_recursive_dir(subpath);
> + if (ret) {
> + goto end;
> + }
> +
> + ret = unlinkat(dir_fd, entry->d_name, AT_REMOVEDIR);
> + if (ret) {
> + PERROR("Unlinking '%s'", entry->d_name);
> + goto end;
> + }
I have removed this unlinkat() to perform an rmdir() at the end to
always remove the "root" on which we are called.
This simplifies the caller in main() and this "case".
> +
> break;
> + }
> case DT_REG:
> - unlinkat(trace_dir_fd, entry->d_name, 0);
> + ret = unlinkat(dir_fd, entry->d_name, 0);
> + if (ret) {
> + PERROR("Unlinking '%s'", entry->d_name);
> + goto end;
> + }
> +
> break;
> default:
> ret = -EINVAL;
> @@ -1089,13 +1116,10 @@ int delete_trace(const char *trace_path)
> }
> }
> end:
> - closeret = closedir(trace_dir);
> + closeret = closedir(dir);
> if (closeret) {
> PERROR("closedir");
> }
> - if (!ret) {
> - ret = rmdir(trace_path);
> - }
> return ret;
> }
>
> @@ -1178,9 +1202,16 @@ int main(int argc, char *argv[])
> has_warning = 1;
>
> /* unlink temporary trace */
> - ret = delete_trace(output_path);
> - if (ret)
> + ret = delete_recursive_dir(output_path);
> + if (ret){
Missing space here ^.
> has_warning = 1;
> + }else{
Missing spaces around "else" here ^.
> + ret = rmdir(output_path);
> + if (ret) {
> + PERROR("Deleting '%s'", output_path);
> + has_warning = 1;
> + }
> + }
> }
> if (has_warning)
> exit(EXIT_FAILURE);
> --
> 2.1.4
>
--
Jérémie Galarneau
EfficiOS Inc.
http://www.efficios.com
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
