CONCPP-71 SQLString destructor should not be virtual

Also internally used class Value is changed to use SQLString, instead of (smart) pointer to it. Added missing SQLString object creation/destruction in that class. Checking in appveyor initial configuration. Added windows test (try) to travis config.
2025-07-20 16:35:25 +00:00 · 2021-02-23 18:12:11 +01:00
parent be22353bc2
commit c06be0a1c9
9 changed files with 281 additions and 63 deletions
--- a/.travis.yml
+++ b/.travis.yml
@ -16,7 +16,7 @@ before_install:
  - .travis/gen-ssl.sh mariadb.example.com tmp
  - export PROJ_PATH=`pwd`
  - export SSLCERT=$PROJ_PATH/tmp
-
+  - case $TRAVIS_OS_NAME in windows) choco install mariadb; esac
 matrix:
  allow_failures:
    - env: DB=mysql:5.6
@ -27,6 +27,8 @@ matrix:
    - env: DB=mariadb:10.5 MAXSCALE_VERSION=2.5.3 MAXSCALE_TEST_DISABLE=true
    - env: SKYSQL=true
    - env: SKYSQL_HA=true MAXSCALE_TEST_DISABLE=true
+    - os: windows
+      env: DB=mariadb:10.5
  include:
    - env: DB=mariadb:10.5 MAXSCALE_VERSION=2.5.3 MAXSCALE_TEST_DISABLE=true
    - env: SKYSQL=true
@ -51,10 +53,13 @@ matrix:
      #- brew install mariadb
    - env: DB=mysql:5.6
    - env: DB=mysql:5.7
+    - os: windows
+      env: DB=mariadb:10.5

 script:
  - if [[ "$DB" == build* ]] ; then .travis/build/build.sh; fi
  - if [[ "$DB" == build* ]] ; then docker build -t build:10.6 --label build .travis/build/; fi
  - if [ "$TRAVIS_OS_NAME" = "linux" ] ; then .travis/script.sh; fi
  - if [ "$TRAVIS_OS_NAME" = "osx" ]; then .travis/osx.sh; fi
+  - if [ "$TRAVIS_OS_NAME" = "windows" ]; then .travis/windows.sh; fi

--- a/.travis/gen-ssl.sh
+++ b/.travis/gen-ssl.sh
@ -111,44 +111,45 @@ main () {
    -out "${tmpKeystoreFile}" \
    -name "mysqlAlias" \
    -passout pass:kspass
+      # there is no keytool on windows (atm)
+  if [ -z "$WINDIR" ]; then
+    # convert PKSC12 to JKS
+    keytool \
+      -importkeystore \
+      -deststorepass kspass \
+      -destkeypass kspass \
+      -destkeystore "${clientKeystoreFile}" \
+      -srckeystore ${tmpKeystoreFile} \
+      -srcstoretype PKCS12 \
+      -srcstorepass kspass \
+      -alias "mysqlAlias"

-  # convert PKSC12 to JKS
-  keytool \
-    -importkeystore \
-    -deststorepass kspass \
-    -destkeypass kspass \
-    -destkeystore "${clientKeystoreFile}" \
-    -srckeystore ${tmpKeystoreFile} \
-    -srcstoretype PKCS12 \
-    -srcstorepass kspass \
-    -alias "mysqlAlias"
-
-  # Now generate a full keystore with the client cert & key + trust certificates
-  log "Generating full client keystore"
-  openssl pkcs12 \
-    -export \
-    -in "${clientCertFile}" \
-    -inkey "${clientKeyFile}" \
-    -out "${pcks12FullKeystoreFile}" \
-    -name "mysqlAlias" \
-    -passout pass:kspass
+    # Now generate a full keystore with the client cert & key + trust certificates
+    log "Generating full client keystore"
+    openssl pkcs12 \
+      -export \
+      -in "${clientCertFile}" \
+      -inkey "${clientKeyFile}" \
+      -out "${pcks12FullKeystoreFile}" \
+      -name "mysqlAlias" \
+      -passout pass:kspass


-  # convert PKSC12 to JKS
-  keytool \
-    -importkeystore \
-    -deststorepass kspass \
-    -destkeypass kspasskey \
-    -deststoretype JKS \
-    -destkeystore "${fullClientKeystoreFile}" \
-    -srckeystore ${pcks12FullKeystoreFile} \
-    -srcstoretype PKCS12 \
-    -srcstorepass kspass \
-    -alias "mysqlAlias"
-
-  log "Generating trustStore"
-  keytool -import -file "${certFile}" -alias CA -keystore "${fullClientKeystoreFile}" -storepass kspass -keypass kspasskey -noprompt
+    # convert PKSC12 to JKS
+    keytool \
+      -importkeystore \
+      -deststorepass kspass \
+      -destkeypass kspasskey \
+      -deststoretype JKS \
+      -destkeystore "${fullClientKeystoreFile}" \
+      -srckeystore ${pcks12FullKeystoreFile} \
+      -srcstoretype PKCS12 \
+      -srcstorepass kspass \
+      -alias "mysqlAlias"

+    log "Generating trustStore"
+    keytool -import -file "${certFile}" -alias CA -keystore "${fullClientKeystoreFile}" -storepass kspass -keypass kspasskey -noprompt
+  fi
  # Clean up CSR file:
  rm "$csrFile"
  rm "$clientReqFile"
--- a/.travis/script.sh
+++ b/.travis/script.sh
@ -56,7 +56,6 @@ else
  export TEST_UID=bob
  export TEST_PASSWORD=
  export TEST_PORT=3305
-  export COMPOSE_FILE=.travis/docker-compose.yml

  if [ -n "$MAXSCALE_VERSION" ] ; then
      # maxscale ports:
--- a/.travis/windows.sh
+++ b/.travis/windows.sh
@ -0,0 +1,99 @@
+#!/bin/bash
+
+set -x
+set -e
+
+###################################################################################################################
+# test different type of configuration
+###################################################################################################################
+
+export TEST_SOCKET=
+export ENTRYPOINT=$PROJ_PATH/.travis/sql
+export ENTRYPOINT_PAM=$PROJ_PATH/.travis/pam
+
+set +x
+
+if [ -n "$SKYSQL" ] || [ -n "$SKYSQL_HA" ]; then
+  if [ -n "$SKYSQL" ]; then
+    ###################################################################################################################
+    # test SKYSQL
+    ###################################################################################################################
+    if [ -z "$SKYSQL_HOST" ] ; then
+      echo "No SkySQL configuration found !"
+      exit 0
+    fi
+
+    export TEST_UID=$SKYSQL_USER
+    export TEST_SERVER=$SKYSQL_HOST
+    export TEST_PASSWORD=$SKYSQL_PASSWORD
+    export TEST_PORT=$SKYSQL_PORT
+    export TEST_SCHEMA=testcpp
+    export TEST_USETLS=true
+
+  else
+
+    ###################################################################################################################
+    # test SKYSQL with replication
+    ###################################################################################################################
+    if [ -z "$SKYSQL_HA" ] ; then
+      echo "No SkySQL HA configuration found !"
+      exit 0
+    fi
+
+    export TEST_UID=$SKYSQL_HA_USER
+    export TEST_SERVER=$SKYSQL_HA_HOST
+    export TEST_PASSWORD=$SKYSQL_HA_PASSWORD
+    export TEST_PORT=$SKYSQL_HA_PORT
+    export TEST_SCHEMA=testcpp
+    export TEST_USETLS=true
+  fi
+
+else
+
+  export TEST_SERVER=localhost
+  export TEST_SCHEMA=testcpp
+  export TEST_UID=root
+  export TEST_PASSWORD=
+  export TEST_PORT=3306
+
+  ls /c/Program\ Files/MariaDB*/bin/
+  MDBPATH=$(find /c/Program\ Files/MariaDB\ 1* -maxdepth 0 | tail -1)
+  PATH=$PATH:$MDBPATH/bin
+
+  set -x
+  # create test database
+  $MDBPATH/bin/mariadb -e "CREATE DATABASE testcpp" --user=$TEST_UID -p"$TEST_PASSWORD"
+
+  mariadb=( mariadb --protocol=TCP -u${TEST_UID} -h${TEST_SERVER} --port=${TEST_PORT} ${TEST_SCHEMA})
+
+  #list ssl certificates
+  ls -lrt ${SSLCERT}
+
+fi
+
+cmake -DCONC_WITH_MSI=OFF -DCONC_WITH_UNIT_TESTS=OFF -DCMAKE_BUILD_TYPE=RelWithDebInfo -DWITH_SSL=SCHANNEL -DTEST_HOST="tcp://$TEST_SERVER:$TEST_PORT" .
+# In Travis we are interested in tests with latest C/C version, while for release we must use only latest release tag
+#git submodule update --remote
+set -x
+cmake --build . --config RelWithDebInfo 
+
+###################################################################################################################
+# run test suite
+###################################################################################################################
+echo "Running tests"
+
+cd test
+
+pwd
+#ls -l cts.sql
+#
+#if [ -n "$SKYSQL" ] ; then
+#  set +x
+#  mysql --protocol=tcp -u$TEST_UID -h$TEST_SERVER --port=$TEST_PORT --ssl -p$TEST_PASSWORD < cts.sql
+#  set -x
+#else
+#  mysql --protocol=tcp -u$TEST_UID -h$TEST_SERVER --port=$TEST_PORT --password=$TEST_PASSWORD < cts.sql
+#fi
+
+ctest -VV
+
--- a/appveyor-download.bat
+++ b/appveyor-download.bat
@ -0,0 +1,16 @@
+@echo off
+set archive=http://ftp.hosteurope.de/mirror/archive.mariadb.org/mariadb-%DB%/winx64-packages/mariadb-%DB%-winx64.msi
+set last=http://mirror.i3d.net/pub/mariadb/mariadb-%DB%/winx64-packages/mariadb-%DB%-winx64.msi
+
+curl -fLsS -o server.msi %archive%
+
+if %ERRORLEVEL% == 0 goto end
+
+curl -fLsS -o server.msi %last%
+if %ERRORLEVEL% == 0  goto end
+
+echo Failure Reason Given is %errorlevel%
+exit /b %errorlevel%
+
+:end
+echo "File found".
--- a/appveyor.yml
+++ b/appveyor.yml
@ -0,0 +1,76 @@
+environment: 
+    global:
+      TEST_UID: root
+      TEST_PASSWORD: root
+      TEST_SERVER: localhost
+      TEST_SCHEMA: test
+    matrix:
+        - APPVEYOR_BUILD_WORKER_IMAGE: Visual Studio 2015
+          DB: 10.5.9
+
+        - APPVEYOR_BUILD_WORKER_IMAGE: Visual Studio 2017
+          DB: 10.4.18
+
+        - APPVEYOR_BUILD_WORKER_IMAGE: Visual Studio 2019
+          DB: 10.5.9
+matrix:
+    allow_failures:
+        - APPVEYOR_BUILD_WORKER_IMAGE: Visual Studio 2015
+
+# scripts that are called at very beginning, before repo cloning
+init:
+    - git config --global core.autocrlf input
+    - wmic cpu get NumberOfCores
+    - wmic ComputerSystem get TotalPhysicalMemory
+
+clone_folder: c:\maodbc
+platform: x64
+configuration: Release
+
+build_script:
+    # build libmariadb separately first because otherwise the Wix installer build might look for files that aren't available yet
+    - cd libmariadb
+    - cmake --build . --config RelWithDebInfo --parallel 2
+    # build conncpp
+    - cd ..
+    - cmake --build . --config RelWithDebInfo --parallel 2
+
+# scripts to run before build
+before_build:
+    - cd c:\maodbc
+    - git submodule init
+    - git submodule update
+    - rm -rf win64
+    - mkdir win64
+    - cd win64
+    - cmake .. -DCONC_WITH_MSI=OFF -DCMAKE_BUILD_TYPE=RelWithDebInfo -DWITH_SIGNCODE=0 -DWITH_SSL=SCHANNEL 
+
+install:
+    # download and install MariaDB Server
+    - cmd: appveyor-download.bat
+    - cmd: msiexec /i server.msi INSTALLDIR=c:\mariadb-server SERVICENAME=mariadb PASSWORD=%TEST_PASSWORD% /qn
+    # create test database
+    - cmd: c:\mariadb-server\bin\mysql.exe -e "CREATE DATABASE testcpp" --user=%TEST_UID% -p%TEST_PASSWORD%
+
+after_build:
+    # install built driver
+    - ps: $msifile = Get-ChildItem $env:APPVEYOR_BUILD_FOLDER\win64\wininstall\mariadb-connector-*.msi | Select-Object -First 1
+    - ps: Push-AppveyorArtifact $msifile.FullName -FileName $msifile.Name
+    - ps: Write $msifile
+    - ps: msiexec /i $msifile INSTALLDIR=c:\mariadb-conncpp /qn 
+
+    - timeout /T 1    
+    # - ps: $blockRdp = $true; iex ((new-object net.webclient).DownloadString('https://raw.githubusercontent.com/appveyor/ci/master/scripts/enable-rdp.ps1'))
+    - cd test
+    - ctest -V
+
+on_finish:
+#  - dir
+#  - dir RelWithDebInfo
+#  - dir wininstall
+#  - dir libmariadb\RelWithDebInfo
+#  - dir release
+#  - dir libmariadb
+#  - dir libmariadb\release
+#  - dir wininstall
+#  - type wininstall\mariadb_odbc.xml
--- a/include/conncpp/SQLString.hpp
+++ b/include/conncpp/SQLString.hpp
@ -44,7 +44,7 @@ public:
  SQLString(const char* str);
  SQLString(const char* str, std::size_t count);
  SQLString();
-  virtual ~SQLString();
+  ~SQLString();

  static constexpr std::size_t npos{static_cast<std::size_t>(-1)};
  const char * c_str() const;
--- a/src/util/Value.cpp
+++ b/src/util/Value.cpp
@ -30,10 +30,6 @@ namespace mariadb

  Value::Value(const Value& other) 
  {
-    if (type == VSTRING)
-    {
-      value.sv.reset(nullptr);
-    }
    type= other.type;
    isPtr= other.isPtr;

@ -46,7 +42,7 @@ namespace mariadb
      switch (type)
      {
      case VSTRING:
-        value.sv.reset(new SQLString(*other.value.sv));
+        new (&value.sv) SQLString(other.value.sv);
        break;
      case VINT32:
        value.iv= other.value.iv;
@ -83,13 +79,13 @@ namespace mariadb

  Value::Value(const SQLString &v) : type(VSTRING), isPtr(false)
  {
-    value.sv.reset(new SQLString(v));
+    new (&value.sv) SQLString(v);
  }


  Value::Value(const char* v) : type(VSTRING), isPtr(false)
  {
-    value.sv.reset(new SQLString(v));
+    new (&value.sv) SQLString(v);
  }


@ -118,15 +114,27 @@ namespace mariadb

  SQLString & Value::operator=(const SQLString &str)
  {
-    isPtr= false;
-    type= VSTRING;
-    value.sv.reset(new SQLString(str));

-    return *value.sv;
+    if (type != VSTRING || isPtr)
+    {
+      type=  VSTRING;
+      isPtr= false;
+      new (&value.sv) SQLString(str);
+    }
+    else
+    {
+      value.sv= str;
+    }
+
+    return value.sv;
  }

  int32_t & Value::operator=(int32_t num)
  {
+    if (type == VSTRING && !isPtr)
+    {
+      value.sv.~SQLString();
+    }
    isPtr= false;
    type= VINT32;
    value.iv= num;
@ -136,6 +144,10 @@ namespace mariadb

  int64_t & Value::operator=(int64_t num)
  {
+    if (type == VSTRING && !isPtr)
+    {
+      value.sv.~SQLString();
+    }
    isPtr= false;
    type= VINT64;
    value.lv= num;
@ -145,6 +157,10 @@ namespace mariadb

  bool & Value::operator=(bool v)
  {
+    if (type == VSTRING && !isPtr)
+    {
+      value.sv.~SQLString();
+    }
    isPtr= false;
    type= VBOOL;
    value.bv= v;
@ -154,6 +170,10 @@ namespace mariadb

  SQLString * Value::operator=(SQLString *str)
  {
+    if (type == VSTRING && !isPtr)
+    {
+      value.sv.~SQLString();
+    }
    isPtr= true;
    type= VSTRING;
    value.pv= str;
@ -173,7 +193,7 @@ namespace mariadb
    case sql::mariadb::Value::VBOOL:
      return (isPtr ? *static_cast<bool*>(value.pv) : value.bv) ? 1 : 0;
    case sql::mariadb::Value::VSTRING:
-      return std::stoi(StringImp::get(isPtr ? *static_cast<SQLString*>(value.pv) : *value.sv));
+      return std::stoi(StringImp::get(isPtr ? *static_cast<SQLString*>(value.pv) : value.sv));
    case sql::mariadb::Value::VNONE:
      // or exception if empty?
      return 0;
@ -202,7 +222,7 @@ namespace mariadb
    case sql::mariadb::Value::VBOOL:
      return (isPtr ? *static_cast<bool*>(value.pv) : value.bv) ? 1 : 0;
    case sql::mariadb::Value::VSTRING:
-      return std::stoll(StringImp::get(isPtr ? *static_cast<SQLString*>(value.pv) : *value.sv));
+      return std::stoll(StringImp::get(isPtr ? *static_cast<SQLString*>(value.pv) : value.sv));
    case sql::mariadb::Value::VNONE:
      return 0;
    }
@ -233,7 +253,7 @@ namespace mariadb
      return (isPtr ? *static_cast<bool*>(value.pv) : value.bv);
    case sql::mariadb::Value::VSTRING:
    {
-      SQLString &str= isPtr ? *static_cast<SQLString*>(value.pv) : *value.sv;
+      const SQLString &str= isPtr ? *static_cast<SQLString*>(value.pv) : value.sv;
      if (str.compare("true") == 0)
      {
        return true;
@ -272,7 +292,7 @@ namespace mariadb
    case sql::mariadb::Value::VBOOL:
      return (isPtr ? *static_cast<bool*>(value.pv) : value.bv) ? "true" : "false";
    case sql::mariadb::Value::VSTRING:
-      return isPtr ? *static_cast<SQLString*>(value.pv) : *value.sv;
+      return isPtr ? *static_cast<SQLString*>(value.pv) : value.sv;
    case sql::mariadb::Value::VNONE:
      return emptyStr;
    }
@ -289,7 +309,7 @@ namespace mariadb
  {
    if (type == VSTRING)
    {
-      return isPtr ? *static_cast<SQLString*>(value.pv) : *value.sv;
+      return isPtr ? *static_cast<SQLString*>(value.pv) : value.sv;
    }

    throw std::runtime_error("Wrong lvalue type requested - the type is not string");
@ -300,7 +320,7 @@ namespace mariadb
  {
    if (type == VSTRING)
    {
-      return StringImp::get(isPtr ? *static_cast<SQLString*>(value.pv) : *value.sv);
+      return StringImp::get(isPtr ? *static_cast<SQLString*>(value.pv) : value.sv);
    }

    throw std::invalid_argument("Wrong lvalue type requested - the type is not string");
@ -344,7 +364,7 @@ namespace mariadb
  {
    if (type == VSTRING)
    {
-      return isPtr ? static_cast<SQLString*>(value.pv) : value.sv.get();
+      return isPtr ? static_cast<SQLString*>(value.pv) : &value.sv;
    }

    throw std::invalid_argument("Wrong lvalue type requested - the type is not string");
@ -355,7 +375,7 @@ namespace mariadb
  {
    if (type == VSTRING)
    {
-      return isPtr ? static_cast<SQLString*>(value.pv)->c_str() : value.sv->c_str();
+      return isPtr ? static_cast<SQLString*>(value.pv)->c_str() : value.sv.c_str();
    }

    throw std::invalid_argument("Wrong lvalue type requested - the type is not string");
@ -370,6 +390,10 @@ namespace mariadb

  void Value::reset()
  {
+    if (type == VSTRING && !isPtr)
+    {
+      value.sv.~SQLString();
+    }
    type= VNONE;
  }

@ -394,14 +418,14 @@ namespace mariadb
            return (static_cast<const SQLString*>(this->value.pv)->compare(*static_cast<const SQLString*>(other.value.pv)) == 0);
          }
          else {
-            return (static_cast<const SQLString*>(this->value.pv)->compare(*other.value.sv) == 0);
+            return (static_cast<const SQLString*>(this->value.pv)->compare(other.value.sv) == 0);
          }
        }
        else {
          //it (other.isPtr) {
          //}
          //else {
-            return this->value.sv->compare(static_cast<const char*>(other)) == 0;
+            return this->value.sv.compare(static_cast<const char*>(other)) == 0;
          //}
        }
      case sql::mariadb::Value::VNONE:
@ -415,9 +439,9 @@ namespace mariadb

  Value::~Value()
  {
-    if (type==VSTRING && !isPtr)
+    if (type == VSTRING && !isPtr)
    {
-      value.sv.reset();
+      value.sv.~SQLString();
    }
  }

--- a/src/util/Value.h
+++ b/src/util/Value.h
@ -48,9 +48,7 @@ private:
    int32_t iv;
    int64_t lv;
    bool    bv;
-    /* Do we really need it here? looks like having something like unique_ptr in a union is not the best idea. should
-       be fairly easy to take care of a plain pointer here */
-    std::unique_ptr<SQLString> sv;
+    SQLString sv;
    void*   pv;

    Variant(): pv(0) {}