Fix: test all close/fclose ret val, fix double close

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>

diff --git a/converter/babeltrace-log.c b/converter/babeltrace-log.c
index d24f011e5ed214785840da4435fe447b4a491c1f..9d87776a8f361ade58a7557215d99fe8a56916fd 100644 (file)
--- a/converter/babeltrace-log.c
+++ b/converter/babeltrace-log.c
@@ -231,10 +231,14 @@ void trace_text(FILE *input, int output)
        ssize_t len;
        char *line = NULL, *nl;
        size_t linesize;
        ssize_t len;
        char *line = NULL, *nl;
        size_t linesize;

+       int ret;

 
        memset(&pos, 0, sizeof(pos));
 
        memset(&pos, 0, sizeof(pos));

-       ctf_init_pos(&pos, output, O_RDWR);
-
+       ret = ctf_init_pos(&pos, output, O_RDWR);
+       if (ret) {
+               fprintf(stderr, "Error in ctf_init_pos\n");
+               return;
+       }

        write_packet_header(&pos, s_uuid);
        write_packet_context(&pos);
        for (;;) {
        write_packet_header(&pos, s_uuid);
        write_packet_context(&pos);
        for (;;) {


@@ -249,7 +253,10 @@ void trace_text(FILE *input, int output)
                        trace_string(line, &pos, strlen(line) + 1);
                }
        }
                        trace_string(line, &pos, strlen(line) + 1);
                }
        }

-       ctf_fini_pos(&pos);
+       ret = ctf_fini_pos(&pos);
+       if (ret) {
+               fprintf(stderr, "Error in ctf_fini_pos\n");
+       }

 }
 
 static
 }
 
 static


@@ -347,7 +354,9 @@ int main(int argc, char **argv)
        print_metadata(metadata_fp);
        trace_text(stdin, fd);
 
        print_metadata(metadata_fp);
        trace_text(stdin, fd);
 

-       close(fd);
+       ret = close(fd);
+       if (ret)
+               perror("close");

        exit(EXIT_SUCCESS);
 
        /* error handling */
        exit(EXIT_SUCCESS);
 
        /* error handling */

diff --git a/formats/bt-dummy/bt-dummy.c b/formats/bt-dummy/bt-dummy.c
index 94edc3be83742ec82b672f157f8678df1670f446..cb912a37d153e02650ccc9c90513680c6bf7f67b 100644 (file)
--- a/formats/bt-dummy/bt-dummy.c
+++ b/formats/bt-dummy/bt-dummy.c
@@ -50,12 +50,13 @@ struct trace_descriptor *bt_dummy_open_trace(const char *path, int flags,
 }
 
 static
 }
 
 static

-void bt_dummy_close_trace(struct trace_descriptor *td)
+int bt_dummy_close_trace(struct trace_descriptor *td)

 {
        struct ctf_text_stream_pos *pos =
                container_of(td, struct ctf_text_stream_pos,
                        trace_descriptor);
        free(pos);
 {
        struct ctf_text_stream_pos *pos =
                container_of(td, struct ctf_text_stream_pos,
                        trace_descriptor);
        free(pos);

+       return 0;

 }
 
 static
 }
 
 static

diff --git a/formats/ctf-text/ctf-text.c b/formats/ctf-text/ctf-text.c
index cbf074e07e76db4f03bfd028803ffbdf94745081..8a8e7c3c104651cfb21ac95e42fa294117a378f2 100644 (file)
--- a/formats/ctf-text/ctf-text.c
+++ b/formats/ctf-text/ctf-text.c
@@ -83,7 +83,7 @@ struct trace_descriptor *ctf_text_open_trace(const char *path, int flags,
                void (*packet_seek)(struct stream_pos *pos, size_t index,
                        int whence), FILE *metadata_fp);
 static
                void (*packet_seek)(struct stream_pos *pos, size_t index,
                        int whence), FILE *metadata_fp);
 static

-void ctf_text_close_trace(struct trace_descriptor *descriptor);
+int ctf_text_close_trace(struct trace_descriptor *descriptor);

 
 static
 rw_dispatch write_dispatch_table[] = {
 
 static
 rw_dispatch write_dispatch_table[] = {


@@ -584,12 +584,18 @@ error:
 }
 
 static
 }
 
 static

-void ctf_text_close_trace(struct trace_descriptor *td)
+int ctf_text_close_trace(struct trace_descriptor *td)

 {
 {

+       int ret;

        struct ctf_text_stream_pos *pos =
                container_of(td, struct ctf_text_stream_pos, trace_descriptor);
        struct ctf_text_stream_pos *pos =
                container_of(td, struct ctf_text_stream_pos, trace_descriptor);

-       fclose(pos->fp);
+       ret = fclose(pos->fp);
+       if (ret) {
+               perror("Error on fclose");
+               return -1;
+       }

        g_free(pos);
        g_free(pos);

+       return 0;

 }
 
 static
 }
 
 static

diff --git a/formats/ctf/ctf.c b/formats/ctf/ctf.c
index d60b4353fc2c0d9bbdaaac45fbfb7e1475dd3b1d..ccc103c586d612cda7877e9168c6c5da22565a8e 100644 (file)
--- a/formats/ctf/ctf.c
+++ b/formats/ctf/ctf.c
@@ -86,7 +86,7 @@ void ctf_set_handle(struct trace_descriptor *descriptor,
                struct bt_trace_handle *handle);
 
 static
                struct bt_trace_handle *handle);
 
 static

-void ctf_close_trace(struct trace_descriptor *descriptor);
+int ctf_close_trace(struct trace_descriptor *descriptor);

 static
 uint64_t ctf_timestamp_begin(struct trace_descriptor *descriptor,
                struct bt_trace_handle *handle, enum bt_clock_type type);
 static
 uint64_t ctf_timestamp_begin(struct trace_descriptor *descriptor,
                struct bt_trace_handle *handle, enum bt_clock_type type);


@@ -548,7 +548,7 @@ error:
        return ret;
 }
 
        return ret;
 }
 

-void ctf_init_pos(struct ctf_stream_pos *pos, int fd, int open_flags)
+int ctf_init_pos(struct ctf_stream_pos *pos, int fd, int open_flags)

 {
        pos->fd = fd;
        if (fd >= 0) {
 {
        pos->fd = fd;
        if (fd >= 0) {


@@ -578,9 +578,10 @@ void ctf_init_pos(struct ctf_stream_pos *pos, int fd, int open_flags)
        default:
                assert(0);
        }
        default:
                assert(0);
        }

+       return 0;

 }
 
 }
 

-void ctf_fini_pos(struct ctf_stream_pos *pos)
+int ctf_fini_pos(struct ctf_stream_pos *pos)

 {
        int ret;
 
 {
        int ret;
 


@@ -592,13 +593,14 @@ void ctf_fini_pos(struct ctf_stream_pos *pos)
                if (ret) {
                        fprintf(stderr, "[error] Unable to unmap old base: %s.\n",
                                strerror(errno));
                if (ret) {
                        fprintf(stderr, "[error] Unable to unmap old base: %s.\n",
                                strerror(errno));

-                       assert(0);
+                       return -1;

                }
        }
        if (pos->packet_cycles_index)
                (void) g_array_free(pos->packet_cycles_index, TRUE);
        if (pos->packet_real_index)
                (void) g_array_free(pos->packet_real_index, TRUE);
                }
        }
        if (pos->packet_cycles_index)
                (void) g_array_free(pos->packet_cycles_index, TRUE);
        if (pos->packet_real_index)
                (void) g_array_free(pos->packet_real_index, TRUE);

+       return 0;

 }
 
 /*
 }
 
 /*


@@ -973,11 +975,20 @@ int ctf_open_trace_metadata_stream_read(struct ctf_trace *td, FILE **fp,
        /* close to flush the buffer */
        ret = babeltrace_close_memstream(buf, &size, out);
        if (ret < 0) {
        /* close to flush the buffer */
        ret = babeltrace_close_memstream(buf, &size, out);
        if (ret < 0) {

+               int closeret;
+

                perror("babeltrace_flush_memstream");
                perror("babeltrace_flush_memstream");

-               fclose(in);
-               return -errno;
+               ret = -errno;
+               closeret = fclose(in);
+               if (closeret) {
+                       perror("Error in fclose");
+               }
+               return ret;
+       }
+       ret = fclose(in);
+       if (ret) {
+               perror("Error in fclose");

        }
        }

-       fclose(in);

        /* open for reading */
        *fp = babeltrace_fmemopen(*buf, strlen(*buf), "rb");
        if (!*fp) {
        /* open for reading */
        *fp = babeltrace_fmemopen(*buf, strlen(*buf), "rb");
        if (!*fp) {


@@ -996,7 +1007,7 @@ int ctf_open_trace_metadata_read(struct ctf_trace *td,
        struct ctf_file_stream *metadata_stream;
        FILE *fp;
        char *buf = NULL;
        struct ctf_file_stream *metadata_stream;
        FILE *fp;
        char *buf = NULL;

-       int ret = 0;
+       int ret = 0, closeret;

 
        metadata_stream = g_new0(struct ctf_file_stream, 1);
        metadata_stream->pos.last_offset = LAST_OFFSET_POISON;
 
        metadata_stream = g_new0(struct ctf_file_stream, 1);
        metadata_stream->pos.last_offset = LAST_OFFSET_POISON;


@@ -1006,7 +1017,7 @@ int ctf_open_trace_metadata_read(struct ctf_trace *td,
        } else {
                fprintf(stderr, "[error] packet_seek function undefined.\n");
                ret = -1;
        } else {
                fprintf(stderr, "[error] packet_seek function undefined.\n");
                ret = -1;

-               goto end_stream;
+               goto end_free;

        }
 
        if (metadata_fp) {
        }
 
        if (metadata_fp) {


@@ -1017,7 +1028,9 @@ int ctf_open_trace_metadata_read(struct ctf_trace *td,
                metadata_stream->pos.fd = openat(td->dirfd, "metadata", O_RDONLY);
                if (metadata_stream->pos.fd < 0) {
                        fprintf(stderr, "Unable to open metadata.\n");
                metadata_stream->pos.fd = openat(td->dirfd, "metadata", O_RDONLY);
                if (metadata_stream->pos.fd < 0) {
                        fprintf(stderr, "Unable to open metadata.\n");

-                       return metadata_stream->pos.fd;
+                       g_free(metadata_stream);
+                       ret = -1;
+                       goto end_free;

                }
 
                fp = fdopen(metadata_stream->pos.fd, "r");
                }
 
                fp = fdopen(metadata_stream->pos.fd, "r");


@@ -1027,6 +1040,8 @@ int ctf_open_trace_metadata_read(struct ctf_trace *td,
                        ret = -errno;
                        goto end_stream;
                }
                        ret = -errno;
                        goto end_stream;
                }

+               /* fd now belongs to fp */
+               metadata_stream->pos.fd = -1;

        }
        if (babeltrace_debug)
                yydebug = 1;
        }
        if (babeltrace_debug)
                yydebug = 1;


@@ -1087,12 +1102,21 @@ end:
        ctf_scanner_free(scanner);
 end_scanner_alloc:
 end_packet_read:
        ctf_scanner_free(scanner);
 end_scanner_alloc:
 end_packet_read:

-       if (fp)
-               fclose(fp);
+       if (fp) {
+               closeret = fclose(fp);
+               if (closeret) {
+                       perror("Error on fclose");
+               }
+       }

        free(buf);
 end_stream:
        free(buf);
 end_stream:

-       if (metadata_stream->pos.fd >= 0)
-               close(metadata_stream->pos.fd);
+       if (metadata_stream->pos.fd >= 0) {
+               closeret = close(metadata_stream->pos.fd);
+               if (closeret) {
+                       perror("Error on metadata stream fd close");
+               }
+       }
+end_free:

        if (ret)
                g_free(metadata_stream);
        return ret;
        if (ret)
                g_free(metadata_stream);
        return ret;


@@ -1482,7 +1506,7 @@ int ctf_open_file_stream_read(struct ctf_trace *td, const char *path, int flags,
                void (*packet_seek)(struct stream_pos *pos, size_t index,
                        int whence))
 {
                void (*packet_seek)(struct stream_pos *pos, size_t index,
                        int whence))
 {

-       int ret, fd;
+       int ret, fd, closeret;

        struct ctf_file_stream *file_stream;
        struct stat statbuf;
 
        struct ctf_file_stream *file_stream;
        struct stat statbuf;
 


@@ -1516,7 +1540,9 @@ int ctf_open_file_stream_read(struct ctf_trace *td, const char *path, int flags,
                goto error_def;
        }
 
                goto error_def;
        }
 

-       ctf_init_pos(&file_stream->pos, fd, flags);
+       ret = ctf_init_pos(&file_stream->pos, fd, flags);
+       if (ret)
+               goto error_def;

        ret = create_trace_definitions(td, &file_stream->parent);
        if (ret)
                goto error_def;
        ret = create_trace_definitions(td, &file_stream->parent);
        if (ret)
                goto error_def;


@@ -1536,11 +1562,17 @@ error_index:
        if (file_stream->parent.trace_packet_header)
                definition_unref(&file_stream->parent.trace_packet_header->p);
 error_def:
        if (file_stream->parent.trace_packet_header)
                definition_unref(&file_stream->parent.trace_packet_header->p);
 error_def:

-       ctf_fini_pos(&file_stream->pos);
+       closeret = ctf_fini_pos(&file_stream->pos);
+       if (closeret) {
+               fprintf(stderr, "Error on ctf_fini_pos\n");
+       }

        g_free(file_stream);
 fd_is_dir_ok:
 fstat_error:
        g_free(file_stream);
 fd_is_dir_ok:
 fstat_error:

-       close(fd);
+       closeret = close(fd);
+       if (closeret) {
+               perror("Error on fd close");
+       }

 error:
        return ret;
 }
 error:
        return ret;
 }


@@ -1551,7 +1583,7 @@ int ctf_open_trace_read(struct ctf_trace *td,
                void (*packet_seek)(struct stream_pos *pos, size_t index,
                        int whence), FILE *metadata_fp)
 {
                void (*packet_seek)(struct stream_pos *pos, size_t index,
                        int whence), FILE *metadata_fp)
 {

-       int ret;
+       int ret, closeret;

        struct dirent *dirent;
        struct dirent *diriter;
        size_t dirent_len;
        struct dirent *dirent;
        struct dirent *diriter;
        size_t dirent_len;


@@ -1624,9 +1656,15 @@ int ctf_open_trace_read(struct ctf_trace *td,
 readdir_error:
        free(dirent);
 error_metadata:
 readdir_error:
        free(dirent);
 error_metadata:

-       close(td->dirfd);
+       closeret = close(td->dirfd);
+       if (closeret) {
+               perror("Error on fd close");
+       }

 error_dirfd:
 error_dirfd:

-       closedir(td->dir);
+       closeret = closedir(td->dir);
+       if (closeret) {
+               perror("Error on closedir");
+       }

 error:
        return ret;
 }
 error:
        return ret;
 }


@@ -1882,17 +1920,28 @@ int ctf_convert_index_timestamp(struct trace_descriptor *tdp)
 }
 
 static
 }
 
 static

-void ctf_close_file_stream(struct ctf_file_stream *file_stream)
+int ctf_close_file_stream(struct ctf_file_stream *file_stream)

 {
 {

-       ctf_fini_pos(&file_stream->pos);
-       close(file_stream->pos.fd);
+       int ret;
+
+       ret = ctf_fini_pos(&file_stream->pos);
+       if (ret) {
+               fprintf(stderr, "Error on ctf_fini_pos\n");
+               return -1;
+       }
+       ret = close(file_stream->pos.fd);
+       if (ret) {
+               perror("Error closing file fd");
+               return -1;
+       }
+       return 0;

 }
 
 static
 }
 
 static

-void ctf_close_trace(struct trace_descriptor *tdp)
+int ctf_close_trace(struct trace_descriptor *tdp)

 {
        struct ctf_trace *td = container_of(tdp, struct ctf_trace, parent);
 {
        struct ctf_trace *td = container_of(tdp, struct ctf_trace, parent);

-       int i;
+       int i, ret;

 
        if (td->streams) {
                for (i = 0; i < td->streams->len; i++) {
 
        if (td->streams) {
                for (i = 0; i < td->streams->len; i++) {


@@ -1906,14 +1955,25 @@ void ctf_close_trace(struct trace_descriptor *tdp)
                                struct ctf_file_stream *file_stream;
                                file_stream = container_of(g_ptr_array_index(stream->streams, j),
                                                struct ctf_file_stream, parent);
                                struct ctf_file_stream *file_stream;
                                file_stream = container_of(g_ptr_array_index(stream->streams, j),
                                                struct ctf_file_stream, parent);

-                               ctf_close_file_stream(file_stream);
+                               ret = ctf_close_file_stream(file_stream);
+                               if (ret)
+                                       return ret;

                        }
                }
        }
        ctf_destroy_metadata(td);
                        }
                }
        }
        ctf_destroy_metadata(td);

-       close(td->dirfd);
-       closedir(td->dir);
+       ret = close(td->dirfd);
+       if (ret) {
+               perror("Error closing dirfd");
+               return ret;
+       }
+       ret = closedir(td->dir);
+       if (ret) {
+               perror("Error closedir");
+               return ret;
+       }

        g_free(td);
        g_free(td);

+       return 0;

 }
 
 static
 }
 
 static

diff --git a/include/babeltrace/ctf/types.h b/include/babeltrace/ctf/types.h
index c1916a4d75b76b7557031e2ca1a96b6febaa1115..1a753084b22b022d0768f3b72bddac267bfb6498 100644 (file)
--- a/include/babeltrace/ctf/types.h
+++ b/include/babeltrace/ctf/types.h
@@ -99,8 +99,8 @@ int ctf_sequence_write(struct stream_pos *pos, struct definition *definition);
 
 void ctf_packet_seek(struct stream_pos *pos, size_t index, int whence);
 
 
 void ctf_packet_seek(struct stream_pos *pos, size_t index, int whence);
 

-void ctf_init_pos(struct ctf_stream_pos *pos, int fd, int open_flags);
-void ctf_fini_pos(struct ctf_stream_pos *pos);
+int ctf_init_pos(struct ctf_stream_pos *pos, int fd, int open_flags);
+int ctf_fini_pos(struct ctf_stream_pos *pos);

 
 /*
  * move_pos - move position of a relative bit offset
 
 /*
  * move_pos - move position of a relative bit offset

diff --git a/include/babeltrace/format.h b/include/babeltrace/format.h
index 5de52fcf73d0ef99a8f401dc50bcfd3c88207e46..a19a0a914d9c91ace187b2f1f1c09db5e9b56296 100644 (file)
--- a/include/babeltrace/format.h
+++ b/include/babeltrace/format.h
@@ -62,7 +62,7 @@ struct format {
                        void (*packet_seek)(struct stream_pos *pos,
                                size_t index, int whence),
                        FILE *metadata_fp);
                        void (*packet_seek)(struct stream_pos *pos,
                                size_t index, int whence),
                        FILE *metadata_fp);

-       void (*close_trace)(struct trace_descriptor *descriptor);
+       int (*close_trace)(struct trace_descriptor *descriptor);

        void (*set_context)(struct trace_descriptor *descriptor,
                        struct bt_context *ctx);
        void (*set_handle)(struct trace_descriptor *descriptor,
        void (*set_context)(struct trace_descriptor *descriptor,
                        struct bt_context *ctx);
        void (*set_handle)(struct trace_descriptor *descriptor,

diff --git a/lib/context.c b/lib/context.c
index 5c9f4b06401d0dc23592d23c66b8bb3751fcd3f2..a044d1c49f833ff104c51065f8f0e79220d4bd7f 100644 (file)
--- a/lib/context.c
+++ b/lib/context.c
@@ -67,7 +67,7 @@ int bt_context_add_trace(struct bt_context *ctx, const char *path,
        struct trace_descriptor *td;
        struct format *fmt;
        struct bt_trace_handle *handle;
        struct trace_descriptor *td;
        struct format *fmt;
        struct bt_trace_handle *handle;

-       int ret;
+       int ret, closeret;

 
        if (!ctx || !format_name || (!path && !stream_list))
                return -EINVAL;
 
        if (!ctx || !format_name || (!path && !stream_list))
                return -EINVAL;


@@ -137,7 +137,10 @@ int bt_context_add_trace(struct bt_context *ctx, const char *path,
        return handle->id;
 
 error:
        return handle->id;
 
 error:

-       fmt->close_trace(td);
+       closeret = fmt->close_trace(td);
+       if (closeret) {
+               fprintf(stderr, "Error in close_trace callback\n");
+       }

 end:
        return ret;
 }
 end:
        return ret;
 }


@@ -145,6 +148,7 @@ end:
 int bt_context_remove_trace(struct bt_context *ctx, int handle_id)
 {
        struct bt_trace_handle *handle;
 int bt_context_remove_trace(struct bt_context *ctx, int handle_id)
 {
        struct bt_trace_handle *handle;

+       int ret;

 
        if (!ctx)
                return -EINVAL;
 
        if (!ctx)
                return -EINVAL;


@@ -157,8 +161,11 @@ int bt_context_remove_trace(struct bt_context *ctx, int handle_id)
        /* Remove from containers */
        trace_collection_remove(ctx->tc, handle->td);
        /* Close the trace */
        /* Remove from containers */
        trace_collection_remove(ctx->tc, handle->td);
        /* Close the trace */

-       handle->format->close_trace(handle->td);
-
+       ret = handle->format->close_trace(handle->td);
+       if (ret) {
+               fprintf(stderr, "Error in close_trace callback\n");
+               return ret;
+       }

        /* Remove and free the handle */
        g_hash_table_remove(ctx->trace_handles,
                        (gpointer) (unsigned long) handle_id);
        /* Remove and free the handle */
        g_hash_table_remove(ctx->trace_handles,
                        (gpointer) (unsigned long) handle_id);