Visual Studio 2008 Woes

[stark025]

#include "storage_sql.h"

#include <string.h>
#include <stdio.h>
#include <stdlib.h>
#define strcasecmp _stricmp
#include <cstring>
#include <cstdlib>
#include <string> 
#include <sstream>
#include <algorithm>
#include "compat/snprintf.h"
#include "common/eventlog.h"
#include "common/util.h"

static const char * sql_backslash_to_underscore(const char * key)
{
  if (!key) {
    ERROR0("got NULL key");
    return NULL;
  }
  
  std::string rep = key;
  std::replace(rep.begin(), rep.end(), '\\', '_');
  return rep.c_str();
}

The above code returns NULL when I pass it strings like "acct\\username", "acct\\passhash" etc. What gives?

[siddhant3s]

Works for me: g++ on Linux

#include<iostream>
#include<string>

static const char * sql_backslash_to_underscore(const char * key)
{
  if (!key) {
    //ERROR0("got NULL key");
    return NULL;
  }
  
  std::string rep = key;
  std::replace(rep.begin(), rep.end(), '\\', '_');
  return rep.c_str();
}
int main()
{
    std::cout<<sql_backslash_to_underscore("acct\\username");
}

siddhant3s@Xion:~$ g++ tester.cpp -o tester
siddhant3s@Xion:~$ ./tester
acct_username

[stark025]

That's the thing: it works on linux but not on Windows.

Anyway fixed it, code is

static const char * sql_backslash_to_underscore(const char * key)
{
  if (!key) {
    ERROR0("got NULL key");
    return NULL;
  }

  std::string rep = key;
  std::replace(rep.begin(),rep.end(),'\\','_');
  return strdup(rep.c_str());
}

[ArkM]

The 1st version was wrong on Windows, Linux or elsewhere. It returns a pointer to deallocated memory of local string data buffer.

The 2nd version works but it's a very dangerous (error-prone) approach to return a pointer to dynamically allocated (by non-standard function strdup) memory. It's so easy to get memory leak and it's too annoying to save returned pointer then free the memory.

Better change this function signature (and code) to

static std::string sql_backslash_to_underscore(const char * key)
{
  std::string rep;
  if (key) {
    rep = key;
    std::replace(rep.begin(),rep.end(),'\\','_');
  }
  return rep;
}

[stark025]

The thing is that the functions calling sql_backslash_to_underscore is expect a const char * instead of a std::string, and I created a copy as a std::string simply so that I can invoke std::replace on it. If there's any function that's identical to std::replace but can work on a const char * and doesn't leak memory, I'd be happy to implement that instead.

[siddhant3s]

>The thing is that the functions calling sql_backslash_to_underscore is expect a
>const char * instead of a std::string
If you don't have trouble changing the calling code, you can make a function call as : sql_backslash_to_underscore("Hello").c_str();

C-type functions which dealing with Cstrings usually let the caller code determine the output. Most of them has prototype as this:
void f(const char* input, char* output);

[stark025]

>The thing is that the functions calling sql_backslash_to_underscore is expect a
>const char * instead of a std::string
If you don't have trouble changing the calling code, you can make a function call as : sql_backslash_to_underscore("Hello").c_str();

C-type functions which dealing with Cstrings usually let the caller code determine the output. Most of them has prototype as this:
void f(const char* input, char* output);

Wouldn't this replicate the problem earlier? I tried it and it returns blank strings again.

[Ancient Dragon]

static const char * sql_backslash_to_underscore(const char * key)
{
  if (!key) {
    ERROR0("got NULL key");
    return NULL;
  }
  char * dup = strdup(key);
  char* ptr;
  while( (ptr = strrchr(dup,'\\')) != NULL)
     *ptr = '_';
  return dup;
}

One problem with the above code is that the caller cannot call free() on a const char* that is returned by the above function. So unless you typecast to remove the const there will be a memory leak.

int main()
{

    const char* ptr = sql_backslash_to_underscore("acct\\username\\Program Files\\Debug");
    cout << ptr << "\n";
    free( (char*)ptr);
}

[siddhant3s]

>Wouldn't this replicate the problem earlier?
No.

std::string sql_backslash_to_underscore(const char * key)
{
  if (!key) {
    //("got NULL key");
    return NULL;
  }
  std::string rep = key;
  std::replace(rep.begin(),rep.end(),'\\','_');
  std::cout<<(void*)rep.c_str();//lets see what is the location of the cstring of rep
  return rep;//note: it is now returning *copy* of rep.
}
int main()
{
const char* a=sql_backslash_to_underscore("acct\\username").c_str();
    std::cout<<std::endl<<(void*)a;//check the location of a
    std::cout<<std::endl<<a ;
    std::cout<<std::endl;
}

[stark025]

This is the code that calls the function:

static int _db_get_tab(const char *key, char **ptab, char **pcol)
{
  static char tab[DB_MAX_ATTRKEY];
  static char col[DB_MAX_ATTRKEY];
  
  std::strncpy(tab, key, DB_MAX_TAB - 1);
  tab[DB_MAX_TAB - 1] = 0;
  
  // no backslash = no table = bad
  if (!std::strchr(tab, '\\')) {
    return -1;
  }
  
  // replace the backslash with a terminator
  *(std::strchr(tab, '\\')) = 0;
  
  // everything after the tab is the column
  std::strncpy(col, key + std::strlen(tab) + 1, DB_MAX_TAB - 1);
  /* since I edited sql_escape_key to allow backslashes, there now is potential
   * for a backslash to have slipped through, so we will convert it to an
   * underscore, as this was the original behavior
   */
  std::strncpy(col, sql_backslash_to_underscore(col), DB_MAX_TAB - 1);
sql_backslash_to_underscore(col), DB_MAX_TAB - 1);] value of col: %s", col);
  // terminate the column
  col[DB_MAX_TAB - 1] = 0;
  *ptab = tab;
  *pcol = col;
  
  return 0;
}

Won't making sql_backslash_to_underscore return strings break this?

[Ancient Dragon]

>>Won't making sql_backslash_to_underscore return strings break this?
Not if you are allowed to change that function. std::strncpy(col, sql_backslash_to_underscore(col).c_str(), DB_MAX_TAB - 1);