How to define a function to return a pointer to a "" value when no matching data is found?

Question

I want my function() to always return a "" string under error conditions else return a string that is converted to string from an unsigned long integer variable.

My initial implementation is as follows:

uint32 cfgVariable_1 = 4;
uint32 cfgVariable_2 = 1;

const char* getCfgVariable (const char* msg)
{
  char* retValue = "";

  if(strcmp("cfgVariable_1", msg)==0)
  {
    // Since I want my function to return a const char* and returning uint32_t is not an option
    sprintf((retValue), "%lu", cfgVariable_1);
    return (const char*)retValue;
  }
  else if(strcmp("cfgVariable_2", msg)==0)
  {
    // Since I want my function to return a const char* and returning uint32_t is not an option
    sprintf((retValue), "%lu", cfgVariable_2);
    return (const char*)retValue;
  }
  else
 {
  //error
 }
 return (const char*) retValue;
}

When the function is called at different instances to get the cfgVariables, I expect my function getCfgVariable() to return "" on error condition, when no match found. Somewhere in code:

 const char* CfgValue = NULL;

 CfgValue = getCfgVariable("cfgVariable_1");

Here CfgValue gets pointed to location which contains 4

later

 const char* CfgValue = NULL;

 CfgValue = getCfgVariable("cfgVariable_3");

I expect to get a "" back but I get 4 instead (CfgValue gets the same address as before).

Fix implemented by me works, but I fail to understand the logic behind it, fix:

const char* getCfgVariable (const char* msg)
{
  const char* defValue = "";
  char* retValue = "\0";

  if(strcmp("cfgVariable_1", msg)==0)
  {
    // Since I want my function to return a const char* and returning uint32_t is not an option
    sprintf((retValue), "%lu", cfgVariable_1);
    return (const char*)retValue;
  }
  else if(strcmp("cfgVariable_2", msg)==0)
  {
    // Since I want my function to return a const char* and returning uint32_t is not an option
    sprintf((retValue), "%lu", cfgVariable_2);
    return (const char*)retValue;
  }
  else
 {
  //error
 }
 return defValue;
}

I see during debugging that defValue and retValue get pointed to two different locations that do not get overwritten. defValue always gets pointed to the same address when its initialized with "" and retValue gets pointed to a different address when initialized with "\0". Can anyone explain the logic behind this ? Is there a better implementation for my use case ?

My Solution after considering the comments:

const char* getCfgVariable (const char* msg)
{
  const char* retValue = "";
  std::ostringstream oss;

  if(!strcmp("cfgVariable_1", msg))
  {
    oss << cfgVariable_1;
  }
  else if(!strcmp("cfgVariable_2", msg))
  {
    oss << cfgVariable_2;
  }
  else
  {
    //error
    return retValue;
  }
  const std::string tmp = oss.str();
  retValue = tmp.c_str();

  return retValue;
}

Thanks for the comments so far and this solution is still open to further improvement suggestions.

The least of the problems here is returning an empty string in some error condition. That's the least that's wrong with the shown code. It is fundamentally flawed, and corrupts a whole bunch of memory. `char* retValue = "";` - this sets up a `char` pointer, that points to exactly one byte of potentially usable memory. Then: `sprintf((retValue), "%lu", cfgVariable_1);` this proceeds and writes who knows how many bytes there. Memory corruption, not to mention the fact that a literal `char` string is, for all practical matters, `const`. You need to rethink this approach, entirely. — Sam Varshavchik, May 01 '19 at 03:26
_Fix implemented by me works, but I fail to understand the logic behind it,_ - I hope you have code reviews of some sort. _Fixes_ submitters by force are generallly frowned upon. — Ted Lyngmo, May 01 '19 at 03:34
@SamVarshavchik is it not possible to implement something fundamentally correct which returns an empty string on error condition ? — Rookie_Coder2318, May 01 '19 at 03:42
Yes. It's certainly possible, but this requires modern C++. This question is tagged "C++", but the shown code is obsolete C code. `strcmp()`, `sprintf()`, are all ancient C library functions. Modern C++ code uses `std::string`, and other C++ classes that correctly handle all storage allocation, safely. Formatted output operations, instead of buffer overflow-prone `sprintf()`. Rough estimate is that the equivalent C++ code with a `std::string` parameter and return values, and `std::ostringstream`, will be about five or six lines of code, a small fraction of this. — Sam Varshavchik, May 01 '19 at 03:59
It is possible, but once you start using raw pointers you have to be even more aware of memory allocation and scope. It isn't safe to return a pointer to memory allocated in the stack, unless it is static, but that has other complications. — Jonathon K, May 01 '19 at 04:00
@SamVarshavchik : please comment on my final solution, that I just added. — Rookie_Coder2318, May 01 '19 at 06:34
Your "final solution" will not work. It returns a pointer to a temporary buffer that gets destroyed when the function returns, so the caller gets a pointer to destroyed memory that will be randomly filled with random garbage. Do yourself a favor, [get a good C++ book](https://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list) and invest some time reading it. C++ is the most complicated general purpose programming language in use today. Trying to get a working program by writing random C++ code, until it compiles, without fully understanding how it works, will not work. — Sam Varshavchik, May 01 '19 at 11:04

robthebloke · Answer 1 · 2019-05-02T01:40:08.033

Constexpr strings such as "\0", "", "cfgVariable_1", etc. These are constant strings in memory compiled into your resulting executable. Attempting to write values into those strings is downright dangerous! In old style C, you'd have to use malloc to allocate a bit of memory to use for your string. This is a real pain to deal with in practice (and not ideal for someone who's learning C++).

A far simpler solution is to start using the C++ string object, std::string (which handles all of the dynamic memory allocation for you!). This should reduce your problem to something a little simpler (and most importantly, safer!):

#include <string>
#include <sstream>

std::string getCfgVariable (const char* const msg)
{
  std::ostringstream oss;
  if(!strcmp("cfgVariable_1", msg))
  {
    oss << cfgVariable_1;
  }
  else 
  if(!strcmp("cfgVariable_2", msg))
  {
    oss << cfgVariable_2;
  }
  return oss.str();
}

Doing this in C, you have 2 choices. Allocate the memory for the returned string, or use a static buffer that is always available (which is what this example does).

uint32 cfgVariable_1 = 4;
uint32 cfgVariable_2 = 1;

const char* getCfgVariable (const char* msg)
{
  static char retValue[32] = {0};

  if(strcmp("cfgVariable_1", msg)==0)
  {
    // Since I want my function to return a const char* and returning uint32_t is not an option
    sprintf(retValue, "%lu", cfgVariable_1);
    return retValue;
  }
  else if(strcmp("cfgVariable_2", msg)==0)
  {
    // Since I want my function to return a const char* and returning uint32_t is not an option
    sprintf(retValue, "%lu", cfgVariable_2);
    return retValue;
  }
  else
 {
  //error
 }
 return retValue;
}

However, because now the retValue is an array fixed in memory, the string returned would only be valid until the next call to getCfgVariable, which could be a little strange....

const char* A = getCfgVariable("cfgVariable_1");
printf("%s\n", A); // prints '4'

const char* B = getCfgVariable("cfgVariable_2");
printf("%s\n", B); // prints '1'
printf("%s\n", A); // now this will print '1', and not '4'. 

const char* C = getCfgVariable("anythingElse");
printf("%s\n", C); // prints ""
printf("%s\n", B); // prints ""
printf("%s\n", A); // aso prints ""

"downright dangerous" - the new "invokes undefined behavior". And fyi, you can do without the string stream as well. [`std::to_string`](https://en.cppreference.com/w/cpp/string/basic_string/to_string) was literally made for situations like this, is implicitly obvious by name alone(reads like a book), and probably more performant than standing up a formatting string stream. — WhozCraig, May 01 '19 at 04:16
@robthebloke : please comment on my final solution, that I just added. — Rookie_Coder2318, May 01 '19 at 06:35
@Rookie_Coder2318 That is not a solution. You are now returning the address of some memory that has been freed. You cannot do that! You must return the allocated memory (and free it later). This is a right pain to manage in C, but is trivial if you return a std::string instead. — robthebloke, May 01 '19 at 06:49
@robthebloke : thanks for your patient responses and I struggle with this problem as I'm new to C++. Anyways my situation is that the function getCfgVariable() is called from a c file so I cannot use the return type std::string for this function, can I ? does ```return tmp.cstr(); ``` in the above "final solution" sound like a fundamentally correct way to return the string as per your suggestion ? Please bare with me and correct me if I'm still wrong. — Rookie_Coder2318, May 01 '19 at 16:20
@robthebloke did some learning and realized what I suggested above is still WRONG/ same as before... I'm planning to use dynamic memory alloc by the caller function which should be a fix, and take care about deleting that memory appropriately. — Rookie_Coder2318, May 02 '19 at 15:29

How to define a function to return a pointer to a "" value when no matching data is found?

1 Answers1