ENGINE-522, ENGINE-523: Fixed DB upgrade issue (we didn't hit it before because it's in some fallback code for when the version is set improperly in the legacy DB) and double-free issues in sync gen code. N.B. We may now have a mem leak in that selfsame code. Am exploring. sync
authorKrista 'DarthMama' Bennett <krista@pep.foundation>
Sat, 09 Mar 2019 11:50:11 +0100
branchsync
changeset 3331d11bc639db5c
parent 3326 f344cad791f2
child 3332 1bdce679b688
child 3335 09af2cf0ac74
ENGINE-522, ENGINE-523: Fixed DB upgrade issue (we didn't hit it before because it's in some fallback code for when the version is set improperly in the legacy DB) and double-free issues in sync gen code. N.B. We may now have a mem leak in that selfsame code. Am exploring.
src/pEpEngine.c
sync/gen_statemachine.ysl2
     1.1 --- a/src/pEpEngine.c	Wed Feb 27 09:30:50 2019 +0100
     1.2 +++ b/src/pEpEngine.c	Sat Mar 09 11:50:11 2019 +0100
     1.3 @@ -498,6 +498,51 @@
     1.4      return 0;
     1.5  }
     1.6  
     1.7 +// TODO: refactor and generalise these two functions if possible.
     1.8 +static int db_contains_table(PEP_SESSION session, const char* table_name) {
     1.9 +    if (!session || !table_name)
    1.10 +        return -1;
    1.11 +    
    1.12 +    // Table names can't be SQL parameters, so we do it this way.
    1.13 +    
    1.14 +    // these two must be the same number.
    1.15 +    char sql_buf[500];
    1.16 +    const size_t max_q_len = 500;
    1.17 +    
    1.18 +    size_t t_size, q_size;
    1.19 +    
    1.20 +    const char* q1 = "SELECT name FROM sqlite_master WHERE type='table' AND name='{"; // 61
    1.21 +    const char* q2 = "'};";       // 3
    1.22 +    
    1.23 +    q_size = 64;
    1.24 +    t_size = strlen(table_name);
    1.25 +    
    1.26 +    size_t query_len = q_size + t_size + 1;
    1.27 +
    1.28 +    if (query_len > max_q_len)
    1.29 +        return -1;
    1.30 +
    1.31 +    strlcpy(sql_buf, q1, max_q_len);
    1.32 +    strlcat(sql_buf, table_name, max_q_len);
    1.33 +    strlcat(sql_buf, q2, max_q_len);
    1.34 +
    1.35 +    sqlite3_stmt *stmt; 
    1.36 +
    1.37 +    sqlite3_prepare_v2(session->db, sql_buf, -1, &stmt, NULL);
    1.38 +
    1.39 +    int retval = 0;
    1.40 +
    1.41 +    int rc = sqlite3_step(stmt);  
    1.42 +    if (rc == SQLITE_DONE || rc == SQLITE_OK || rc == SQLITE_ROW) {
    1.43 +        retval = 1;
    1.44 +    }
    1.45 +
    1.46 +    sqlite3_finalize(stmt);      
    1.47 +        
    1.48 +    return retval;
    1.49 +        
    1.50 +}
    1.51 +
    1.52  static int table_contains_column(PEP_SESSION session, const char* table_name,
    1.53                                                        const char* col_name) {
    1.54  
    1.55 @@ -843,11 +888,13 @@
    1.56          assert(int_result == SQLITE_OK);
    1.57  
    1.58          
    1.59 -        // Sometimes the user_version wasn't set correctly. Check to see if this
    1.60 -        // is really necessary...
    1.61 +        // Sometimes the user_version wasn't set correctly. 
    1.62          if (version == 1) {
    1.63              bool version_changed = true;
    1.64 -            if (table_contains_column(_session, "identity", "timestamp") > 0) {
    1.65 +            if (db_contains_table(_session, "social_graph") > 0) {
    1.66 +                version = 9;
    1.67 +            }            
    1.68 +            else if (table_contains_column(_session, "identity", "timestamp") > 0) {
    1.69                  version = 8;
    1.70              }            
    1.71              if (table_contains_column(_session, "person", "is_pEp_user") > 0) {
    1.72 @@ -1100,7 +1147,7 @@
    1.73                      "    CONSTRAINT fk_own_identity\n"
    1.74                      "       FOREIGN KEY(own_address, own_userid)\n" 
    1.75                      "       REFERENCES identity(address, user_id)\n"
    1.76 -                    "       ON DELETE CASCADE ON UPDATE CASCADE,\n"
    1.77 +                    "       ON DELETE CASCADE ON UPDATE CASCADE\n"
    1.78                      ");\n"
    1.79                      "create table if not exists revocation_contact_list (\n"
    1.80                      "   fpr text not null references pgp_keypair (fpr)\n"
     2.1 --- a/sync/gen_statemachine.ysl2	Wed Feb 27 09:30:50 2019 +0100
     2.2 +++ b/sync/gen_statemachine.ysl2	Sat Mar 09 11:50:11 2019 +0100
     2.3 @@ -302,8 +302,7 @@
     2.4                  return PEP_STATUS_OK;
     2.5  
     2.6              the_end:
     2.7 -                free_«@name»_event(ev);
     2.8 -                free_«@name»_message(msg);
     2.9 +                free_«@name»_event(ev); // msg gets freed here
    2.10                  return status;
    2.11              }
    2.12  
    2.13 @@ -366,8 +365,7 @@
    2.14                  return PEP_STATUS_OK;
    2.15  
    2.16              the_end:
    2.17 -                free_«@name»_event(ev);
    2.18 -                free_«@name»_message(msg);
    2.19 +                free_«@name»_event(ev); // msg gets freed here
    2.20                  return status;
    2.21              }
    2.22  
    2.23 @@ -559,7 +557,7 @@
    2.24                  status = «@name»_driver(session, fsm, event);
    2.25  
    2.26              the_end:
    2.27 -                free_«@name»_event(ev);
    2.28 +                //free_«@name»_event(ev); // FIXME: We don't own this pointer. Are we sure it gets freed externally?
    2.29                  return status;
    2.30              }
    2.31  
    2.32 @@ -1013,4 +1011,3 @@
    2.33          | }
    2.34      }
    2.35  }
    2.36 -