0

I am facing an issue with the legacy code. The legacy code uses a char array whose size is 1024 and is being passed to strtok c function.

char record[1024] = { '\0' };
char *token = NULL;
strcpy(record, inputString);// 'inputString' very large string.
token = strtok(record, DELIMETER);

And now because of new requirement, I have to modify the size to 20000 bytes and in many places they have declared this type (record)of local variable.

Now we are compiling the code with C++11 compiler and since we are using C++11 compiler, I was planning to modify the char array to unique_ptr as shown...

#define MAX_RECORD 50000
auto record = std::make_unique<char[]>(MAX_RECORD * 4);
char *token = NULL;
strcpy(record.get(), inputString);
token = strtok(record.get(), DELIMETER);

My question is, can I pass the unique_ptr to strtok function as the record variable gets modified inside the strtok function?

Thanks in advance.

NJMR
  • 1,886
  • 1
  • 27
  • 46
  • 3
    Why not use `std::vector` and pass its `.data()`? – Joker_vD Sep 08 '17 at 16:11
  • 3
    Any thought about using a [modern approach](https://stackoverflow.com/questions/236129/most-elegant-way-to-split-a-string) instead of a legacy C function? – NathanOliver Sep 08 '17 at 16:12
  • 1
    What is `strcopy(record.get(), record);` supposed to do? – Passer By Sep 08 '17 at 16:12
  • I would use `std::string` rather than allocated memory in a smart pointer. – Galik Sep 08 '17 at 16:15
  • @PasserBy: Sorry copy-paste mistake. – NJMR Sep 08 '17 at 16:18
  • @NathanOliver: With the modern approach, I have to make more changes to the code and hence more testing and requires more time. But will propose this to my team. – NJMR Sep 08 '17 at 16:20
  • Tbh a stack bound array is probably as good as it gets. It is high performance (compared to dynamic allocation) and cleans itself up (no memory leaks). I would not change it unless you want to modernize the whole function. – Galik Sep 08 '17 at 16:29
  • If you are planning to use C++11 and refactor the code accordingly, why don't you ditch the C leftovers entirely and use `std::string` instead of char array as well as `std::getline()` together with `std::stringstream` instead of `strtok()`? Another solution (inferior to the the first one IMO) would be using `std::vector` instead of unique pointer, which will save you all these `get()` and `make_unique()` calls. – KjMag Sep 08 '17 at 16:31

2 Answers2

4

Passing a unique_ptr to a function represents a transfer of ownership of that memory to the function. And in a way, that's kind of what strtok does, since it will keep hold of that pointer beyond the execution of that function. But since it's a C function, it cannot effectively represent that in its interface.

What you have to do is what you're doing now: manage memory for strtok. You need to keep that unique_ptr alive for as long as your code requires strtok to be able to access it. In the previous code, it was stack memory. In your code, it's heap memory owned by a stack object. Either way, the memory ownership semantics work.

Unless the original code was broken (ie: someone called strtok with NULL outside the scope of the array). In which case your new code will be broken in the same way. Though likely, it will break more visibly.

Nicol Bolas
  • 449,505
  • 63
  • 781
  • 982
1

My question is, can I pass the unique_ptr to strtok function as the record variable gets modified inside the strtok function?

To get proper answer, you need to ask correct question. Statement that strtok() modifies record variable is incorrect, it may modify memory, where record points to, but it cannot modify variable record itself. That different things and it is important here. So answer to corrected question is no, you cannot pass std::unique_ptr directly to strtok() obviously, as it is C++ object and strtok() is C function. But yes, you can pass underlying pointer managed by std::unique_ptr, and you need to maintain lifetime of std::unique_ptr object itself long enough that it would not break strtok() requirements of that memory.

About code, it is not clear why you need std::unique_ptr and strcpy() at all:

std::string record = inputString;  // make a copy, if you cannot modify inputString, or pass inputString itself
token = strtok(record.data(), DELIMETER);

in this case you just need to maintain lifetime of variable record the same way, but you do not need to deal with strcpy() and memory allocation.

Slava
  • 43,454
  • 1
  • 47
  • 90