From 3ac1f8f135cb64af307ed87b971e5211a7986129 Mon Sep 17 00:00:00 2001 From: Anthony Scemama Date: Wed, 3 Mar 2021 19:20:01 +0100 Subject: [PATCH 1/3] Locking asked from the front to the back --- src/trexio.c | 71 +++++++++++++++++++++++++++++++++++++++---- src/trexio.h | 1 + src/trexio.org | 73 +++++++++++++++++++++++++++++++++++++++++---- src/trexio_hdf5.org | 2 +- src/trexio_text.c | 30 ++++++++++++------- src/trexio_text.h | 4 +++ src/trexio_text.org | 43 +++++++++++++++++++------- 7 files changed, 192 insertions(+), 32 deletions(-) diff --git a/src/trexio.c b/src/trexio.c index 627f8e6..2d940d4 100644 --- a/src/trexio.c +++ b/src/trexio.c @@ -31,6 +31,7 @@ trexio_t* trexio_open(const char* file_name, const char mode, const back_end_t b trexio_t* result = NULL; + /* Allocate data structures */ switch (back_end) { case TREXIO_TEXT: @@ -47,8 +48,10 @@ trexio_t* trexio_open(const char* file_name, const char mode, const back_end_t b */ } - /* TODO: Error handling */ - assert (result != NULL); + assert (result != NULL); /* TODO: Error handling */ + + + /* Data for the parent type */ result->file_name = (char*) calloc(strlen(file_name)+1,sizeof(char)); strcpy(result->file_name, file_name); @@ -57,7 +60,11 @@ trexio_t* trexio_open(const char* file_name, const char mode, const back_end_t b int irc = pthread_mutex_init ( &(result->thread_lock), NULL); assert (irc == 0); - trexio_exit_code rc = TREXIO_FAILURE; + trexio_exit_code rc; + + /* Back end initialization */ + + rc = TREXIO_FAILURE; switch (back_end) { @@ -76,6 +83,31 @@ trexio_t* trexio_open(const char* file_name, const char mode, const back_end_t b } if (rc != TREXIO_SUCCESS) return NULL; + + /* File locking */ + + rc = TREXIO_FAILURE; + + switch (back_end) { + + case TREXIO_TEXT: + rc = trexio_text_lock(result); + break; + + case TREXIO_HDF5: + rc = TREXIO_SUCCESS; +/* + rc = trexio_hdf5_lock(result); + break; + + case TREXIO_JSON: + rc = trexio_json_lock(result); + break; +*/ + } + if (rc != TREXIO_SUCCESS) return NULL; + + return result; } @@ -85,6 +117,7 @@ trexio_exit_code trexio_close(trexio_t* file) { trexio_exit_code rc; + /* Terminate the back end */ switch (file->back_end) { case TREXIO_TEXT: @@ -106,14 +139,42 @@ trexio_exit_code trexio_close(trexio_t* file) { if (rc != TREXIO_SUCCESS) { return TREXIO_FAILURE; } + + + /* File unlocking */ + + rc = TREXIO_FAILURE; + + switch (file->back_end) { + + case TREXIO_TEXT: + rc = trexio_text_unlock(file); + break; + + case TREXIO_HDF5: + rc = TREXIO_SUCCESS; +/* + rc = trexio_hdf5_unlock(file); + break; + + case TREXIO_JSON: + rc = trexio_json_unlock(file); + break; +*/ + } + if (rc != TREXIO_SUCCESS) return TREXIO_FAILURE; + + + /* Terminate front end */ free(file->file_name); file->file_name = NULL; int irc = pthread_mutex_destroy( &(file->thread_lock) ); - assert (irc == 0); - free(file); + + if (irc != 0) return TREXIO_ERRNO; + return TREXIO_SUCCESS; } diff --git a/src/trexio.h b/src/trexio.h index d00c1ff..26aad39 100644 --- a/src/trexio.h +++ b/src/trexio.h @@ -21,6 +21,7 @@ typedef int32_t trexio_exit_code; #define TREXIO_INVALID_ARG_5 ( (trexio_exit_code) 5 ) #define TREXIO_END ( (trexio_exit_code) 10 ) #define TREXIO_READONLY ( (trexio_exit_code) 11 ) +#define TREXIO_ERRNO ( (trexio_exit_code) 12 ) typedef int32_t back_end_t; diff --git a/src/trexio.org b/src/trexio.org index 050df1c..0bc5bac 100644 --- a/src/trexio.org +++ b/src/trexio.org @@ -78,6 +78,7 @@ typedef int32_t trexio_exit_code; #define TREXIO_INVALID_ARG_5 ( (trexio_exit_code) 5 ) #define TREXIO_END ( (trexio_exit_code) 10 ) #define TREXIO_READONLY ( (trexio_exit_code) 11 ) +#define TREXIO_ERRNO ( (trexio_exit_code) 12 ) #+end_src ** Back ends @@ -168,6 +169,7 @@ trexio_t* trexio_open(const char* file_name, const char mode, const back_end_t b trexio_t* result = NULL; + /* Allocate data structures */ switch (back_end) { case TREXIO_TEXT: @@ -184,8 +186,10 @@ trexio_t* trexio_open(const char* file_name, const char mode, const back_end_t b ,*/ } - /* TODO: Error handling */ - assert (result != NULL); + assert (result != NULL); /* TODO: Error handling */ + + + /* Data for the parent type */ result->file_name = (char*) calloc(strlen(file_name)+1,sizeof(char)); strcpy(result->file_name, file_name); @@ -194,7 +198,11 @@ trexio_t* trexio_open(const char* file_name, const char mode, const back_end_t b int irc = pthread_mutex_init ( &(result->thread_lock), NULL); assert (irc == 0); - trexio_exit_code rc = TREXIO_FAILURE; + trexio_exit_code rc; + + /* Back end initialization */ + + rc = TREXIO_FAILURE; switch (back_end) { @@ -213,6 +221,31 @@ trexio_t* trexio_open(const char* file_name, const char mode, const back_end_t b } if (rc != TREXIO_SUCCESS) return NULL; + + /* File locking */ + + rc = TREXIO_FAILURE; + + switch (back_end) { + + case TREXIO_TEXT: + rc = trexio_text_lock(result); + break; + + case TREXIO_HDF5: + rc = TREXIO_SUCCESS; +/* + rc = trexio_hdf5_lock(result); + break; + + case TREXIO_JSON: + rc = trexio_json_lock(result); + break; +,*/ + } + if (rc != TREXIO_SUCCESS) return NULL; + + return result; } #+end_src @@ -230,6 +263,7 @@ trexio_exit_code trexio_close(trexio_t* file) { trexio_exit_code rc; + /* Terminate the back end */ switch (file->back_end) { case TREXIO_TEXT: @@ -251,14 +285,42 @@ trexio_exit_code trexio_close(trexio_t* file) { if (rc != TREXIO_SUCCESS) { return TREXIO_FAILURE; } + + + /* File unlocking */ + + rc = TREXIO_FAILURE; + + switch (file->back_end) { + + case TREXIO_TEXT: + rc = trexio_text_unlock(file); + break; + + case TREXIO_HDF5: + rc = TREXIO_SUCCESS; +/* + rc = trexio_hdf5_unlock(file); + break; + + case TREXIO_JSON: + rc = trexio_json_unlock(file); + break; +,*/ + } + if (rc != TREXIO_SUCCESS) return TREXIO_FAILURE; + + + /* Terminate front end */ free(file->file_name); file->file_name = NULL; int irc = pthread_mutex_destroy( &(file->thread_lock) ); - assert (irc == 0); - free(file); + + if (irc != 0) return TREXIO_ERRNO; + return TREXIO_SUCCESS; } @@ -618,5 +680,4 @@ trexio_exit_code trexio_buffered_write_rdm_two_e(trexio_t* file, const int64_t o - [ ] Error handling with errno - [ ] HDF5 back-end - [ ] JSON back-end - - [ ] File locking with flock - [ ] Caching of the struct saving last modification date in structs diff --git a/src/trexio_hdf5.org b/src/trexio_hdf5.org index 97d2b91..c37925f 100644 --- a/src/trexio_hdf5.org +++ b/src/trexio_hdf5.org @@ -526,7 +526,7 @@ trexio_exit_code trexio_hdf5_write_nucleus_num(const trexio_t* file, const uint6 if (nucleus->num != 0) { printf("%ld -> %ld %s \n", num, nucleus->num, - "This variable alreasy exists. Overwriting it is not supported"); + "This variable already exists. Overwriting it is not supported"); trexio_hdf5_free_nucleus(nucleus); return TREXIO_FAILURE; } diff --git a/src/trexio_text.c b/src/trexio_text.c index 5b795f0..d18dc3b 100644 --- a/src/trexio_text.c +++ b/src/trexio_text.c @@ -38,7 +38,18 @@ trexio_exit_code trexio_text_init(trexio_t* file) { assert (f->lock_file > 0); free(file_name); - /* Lock the directory for the current process */ + f->nucleus = NULL; + f->electron= NULL; + f->rdm = NULL; + + return TREXIO_SUCCESS; +} + +trexio_exit_code trexio_text_lock(trexio_t* file) { + if (file == NULL) return TREXIO_INVALID_ARG_1; + + trexio_text_t* f = (trexio_text_t*) file; + struct flock fl; fl.l_type = F_WRLCK; @@ -50,20 +61,12 @@ trexio_exit_code trexio_text_init(trexio_t* file) { int rc = fcntl(f->lock_file, F_SETLKW, &fl); if (rc == -1) return TREXIO_FAILURE; - trexio_text_t* f_child = (trexio_text_t*) file; - - f_child->nucleus = NULL; - f_child->electron= NULL; - f_child->rdm = NULL; - return TREXIO_SUCCESS; } trexio_exit_code trexio_text_finalize(trexio_t* file) { if (file == NULL) return TREXIO_INVALID_ARG_1; - trexio_text_t* f = (trexio_text_t*) file; - trexio_exit_code rc; rc = trexio_text_free_nucleus( (trexio_text_t*) file); assert (rc == TREXIO_SUCCESS); @@ -71,7 +74,14 @@ trexio_exit_code trexio_text_finalize(trexio_t* file) { rc = trexio_text_free_rdm( (trexio_text_t*) file); assert (rc == TREXIO_SUCCESS); - /* Release lock */ + return TREXIO_SUCCESS; +} + +trexio_exit_code trexio_text_unlock(trexio_t* file) { + if (file == NULL) return TREXIO_INVALID_ARG_1; + + trexio_text_t* f = (trexio_text_t*) file; + struct flock fl; fl.l_type = F_WRLCK; diff --git a/src/trexio_text.h b/src/trexio_text.h index 593ea1a..ea8c1c4 100644 --- a/src/trexio_text.h +++ b/src/trexio_text.h @@ -56,8 +56,12 @@ typedef struct trexio_text_s { trexio_exit_code trexio_text_init(trexio_t* file); +trexio_exit_code trexio_text_lock(trexio_t* file); + trexio_exit_code trexio_text_finalize(trexio_t* file); +trexio_exit_code trexio_text_unlock(trexio_t* file); + nucleus_t* trexio_text_read_nucleus(trexio_text_t* file); trexio_exit_code trexio_text_flush_nucleus(const trexio_text_t* file); diff --git a/src/trexio_text.org b/src/trexio_text.org index 208cd5e..649309d 100644 --- a/src/trexio_text.org +++ b/src/trexio_text.org @@ -136,7 +136,25 @@ trexio_exit_code trexio_text_init(trexio_t* file) { assert (f->lock_file > 0); free(file_name); - /* Lock the directory for the current process */ + f->nucleus = NULL; + f->electron= NULL; + f->rdm = NULL; + + return TREXIO_SUCCESS; +} + + #+end_src + + #+begin_src c :tangle trexio_text.h +trexio_exit_code trexio_text_lock(trexio_t* file); + #+end_src + + #+begin_src c :tangle trexio_text.c +trexio_exit_code trexio_text_lock(trexio_t* file) { + if (file == NULL) return TREXIO_INVALID_ARG_1; + + trexio_text_t* f = (trexio_text_t*) file; + struct flock fl; fl.l_type = F_WRLCK; @@ -148,12 +166,6 @@ trexio_exit_code trexio_text_init(trexio_t* file) { int rc = fcntl(f->lock_file, F_SETLKW, &fl); if (rc == -1) return TREXIO_FAILURE; - trexio_text_t* f_child = (trexio_text_t*) file; - - f_child->nucleus = NULL; - f_child->electron= NULL; - f_child->rdm = NULL; - return TREXIO_SUCCESS; } @@ -168,8 +180,6 @@ trexio_exit_code trexio_text_finalize(trexio_t* file); trexio_exit_code trexio_text_finalize(trexio_t* file) { if (file == NULL) return TREXIO_INVALID_ARG_1; - trexio_text_t* f = (trexio_text_t*) file; - trexio_exit_code rc; rc = trexio_text_free_nucleus( (trexio_text_t*) file); assert (rc == TREXIO_SUCCESS); @@ -177,7 +187,20 @@ trexio_exit_code trexio_text_finalize(trexio_t* file) { rc = trexio_text_free_rdm( (trexio_text_t*) file); assert (rc == TREXIO_SUCCESS); - /* Release lock */ + return TREXIO_SUCCESS; +} + #+end_src + + #+begin_src c :tangle trexio_text.h +trexio_exit_code trexio_text_unlock(trexio_t* file); + #+end_src + + #+begin_src c :tangle trexio_text.c +trexio_exit_code trexio_text_unlock(trexio_t* file) { + if (file == NULL) return TREXIO_INVALID_ARG_1; + + trexio_text_t* f = (trexio_text_t*) file; + struct flock fl; fl.l_type = F_WRLCK; From 9494f02e370236d5d86b5e338f3e1f4d7b6cf98d Mon Sep 17 00:00:00 2001 From: q-posev Date: Thu, 4 Mar 2021 09:41:00 +0100 Subject: [PATCH 2/3] append on non-existing file should fail --- src/trexio_hdf5.c | 18 +++++++----------- src/trexio_hdf5.org | 16 ++++++---------- 2 files changed, 13 insertions(+), 21 deletions(-) diff --git a/src/trexio_hdf5.c b/src/trexio_hdf5.c index d99d341..8c46932 100644 --- a/src/trexio_hdf5.c +++ b/src/trexio_hdf5.c @@ -94,11 +94,11 @@ trexio_exit_code trexio_hdf5_init(trexio_t* file) { switch (file->mode) { case 'r': - // reading non-existing file -> error - return TREXIO_FAILURE; case 'a': + // reading or appending non-existing file -> error + return TREXIO_FAILURE; case 'w': - // appending or writing non-existing file -> create it + // writing non-existing file -> create it f->file_id = H5Fcreate(file->file_name, H5F_ACC_EXCL, H5P_DEFAULT, H5P_DEFAULT); break; } @@ -107,15 +107,11 @@ trexio_exit_code trexio_hdf5_init(trexio_t* file) { /* Create or open groups in the hdf5 file assuming that they exist if file exists */ switch (file->mode) { + // the switch for 'r'/'a' is reached only if file exists case 'r': case 'a': - if (f_exists == 1) { - f->nucleus_group = H5Gopen(f->file_id, NUCLEUS_GROUP_NAME, H5P_DEFAULT); - //f->electron_group = H5Gopen(f->file_id, ELECTRON_GROUP_NAME, H5P_DEFAULT); - } else { - f->nucleus_group = H5Gcreate(f->file_id, NUCLEUS_GROUP_NAME, H5P_DEFAULT, H5P_DEFAULT, H5P_DEFAULT); - //f->electron_group = H5Gcreate(f->file_id, ELECTRON_GROUP_NAME, H5P_DEFAULT, H5P_DEFAULT, H5P_DEFAULT); - } + f->nucleus_group = H5Gopen(f->file_id, NUCLEUS_GROUP_NAME, H5P_DEFAULT); + //f->electron_group = H5Gopen(f->file_id, ELECTRON_GROUP_NAME, H5P_DEFAULT); break; case 'w': f->nucleus_group = H5Gcreate(f->file_id, NUCLEUS_GROUP_NAME, H5P_DEFAULT, H5P_DEFAULT, H5P_DEFAULT); @@ -375,7 +371,7 @@ trexio_exit_code trexio_hdf5_write_nucleus_num(const trexio_t* file, const uint6 if (nucleus->num != 0) { printf("%ld -> %ld %s \n", num, nucleus->num, - "This variable alreasy exists. Overwriting it is not supported"); + "This variable already exists. Overwriting it is not supported"); trexio_hdf5_free_nucleus(nucleus); return TREXIO_FAILURE; } diff --git a/src/trexio_hdf5.org b/src/trexio_hdf5.org index c37925f..d146d3a 100644 --- a/src/trexio_hdf5.org +++ b/src/trexio_hdf5.org @@ -185,11 +185,11 @@ trexio_exit_code trexio_hdf5_init(trexio_t* file) { switch (file->mode) { case 'r': - // reading non-existing file -> error - return TREXIO_FAILURE; case 'a': + // reading or appending non-existing file -> error + return TREXIO_FAILURE; case 'w': - // appending or writing non-existing file -> create it + // writing non-existing file -> create it f->file_id = H5Fcreate(file->file_name, H5F_ACC_EXCL, H5P_DEFAULT, H5P_DEFAULT); break; } @@ -198,15 +198,11 @@ trexio_exit_code trexio_hdf5_init(trexio_t* file) { /* Create or open groups in the hdf5 file assuming that they exist if file exists */ switch (file->mode) { + // the switch for 'r'/'a' is reached only if file exists case 'r': case 'a': - if (f_exists == 1) { - f->nucleus_group = H5Gopen(f->file_id, NUCLEUS_GROUP_NAME, H5P_DEFAULT); - //f->electron_group = H5Gopen(f->file_id, ELECTRON_GROUP_NAME, H5P_DEFAULT); - } else { - f->nucleus_group = H5Gcreate(f->file_id, NUCLEUS_GROUP_NAME, H5P_DEFAULT, H5P_DEFAULT, H5P_DEFAULT); - //f->electron_group = H5Gcreate(f->file_id, ELECTRON_GROUP_NAME, H5P_DEFAULT, H5P_DEFAULT, H5P_DEFAULT); - } + f->nucleus_group = H5Gopen(f->file_id, NUCLEUS_GROUP_NAME, H5P_DEFAULT); + //f->electron_group = H5Gopen(f->file_id, ELECTRON_GROUP_NAME, H5P_DEFAULT); break; case 'w': f->nucleus_group = H5Gcreate(f->file_id, NUCLEUS_GROUP_NAME, H5P_DEFAULT, H5P_DEFAULT, H5P_DEFAULT); From a3e2a3920d3774daba4f5e976d7689778ee64d79 Mon Sep 17 00:00:00 2001 From: q-posev Date: Thu, 4 Mar 2021 09:41:27 +0100 Subject: [PATCH 3/3] modify the test to fail when appending on non-existing file --- src/test.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/test.c b/src/test.c index c65e088..37e3524 100644 --- a/src/test.c +++ b/src/test.c @@ -120,15 +120,11 @@ int test_h5read() { file2 = trexio_open(file_name2, 'r', TREXIO_HDF5); assert (file2 == NULL); - // test appending non-existing file, should create it - const char* file_name3 = "test_append.h5"; + // test appending non-existing file, should fail and return NULL trexio_t* file3 = NULL; - file3 = trexio_open(file_name3, 'a', TREXIO_HDF5); - assert (file3 != NULL); - - rc = trexio_close(file3); - assert (rc == TREXIO_SUCCESS); + file3 = trexio_open(file_name2, 'a', TREXIO_HDF5); + assert (file3 == NULL); free(coord);