Passing / modifying char* to/ by function.

Vaclav_

My task is to make a function to append int to char*, using C style and end up with char*. No string etc. I have it partially figured out - see attached code. I have some understanding how passing by value works, but not sure how to actually code / use the strcat - the "all concatenated / done " part and use it in main code.

char * CLASS_LCM1602::BuildMessage(char *&title, int value) {

// convert int to char
char buffer\[100\];
snprintf(buffer, sizeof(buffer), "%d", value);

#ifdef DEBUG
cout << " conversion int to char buffer " << +buffer << endl; //TOK
#endif

char FullMessage\[100\];
// why is this needed ?? strcat won't execute without strcpy first
strcpy(FullMessage, title);
cout << "title copy " << FullMessage << endl;
//append buffer
strcat(FullMessage, buffer);
cout << "full message " << FullMessage << endl; // TOK

this is where I need some help
this does not makes sense to me
it compiles , does not execute

strcpy(title, FullMessage);
cout << "title " << title << endl;

exit(1);  // temp exit 

return FullMessage;  // return not used , need to compile

}

The function call

    char \*title = "NEW TEST";
int  value  = 100;
char \*message = i2c.BuildMessage(title, value); // message not used for now

here is the desired result code
never gets here

cout << " pass full message "<< +title << endl;

#endif

Any ( factual ) help will be appreciated. Cheers Vaclav

CPallini

Please state clearly what are your requirements: you wrote

Quote:

y task is to make a function to append int to char*, using C style and end up with char*

but then you are using C++ stuff and the snprintf, strcopy, etc. functions. In a pure-C approach you have (basically) two alternatives:

Passing a buffer containing the source string, but also large enought to contain the target one.
Pass only the source string, and return a freshly allocated buffer containing the target one. Then it is caller task to eventually release the memory.

Lost User

You could reduce the number of buffers you are using, as that seems to complicate your code. You are also trying to return FullMessage which is a temporary buffer inside the function. So as soon as you return from the function that buffer's memory space is free to be overwritten. And what is that exit(1) call there for? No wonder you never return from your function. You need something simple like (using C style only):

#define MAX_BUFFER 100
char * CLASS_LCM1602::BuildMessage(char *title, int value)
{
// allocate a new buffer for the return message
char* buffer = (char*)malloc(MAX_BUFFER );
snprintf(buffer, MAX_BUFFER , "%s %d", title, value); // this assumes 100 characters is enough

return buffer;

}

And then the call is:

char *message = i2c.BuildMessage(title, value);
printf("%s\n", message);
free(message); // release the buffer when finished with

CPallini

Quote:

snprintf(buffer, sizeof(buffer), "%s %d", title, value); // this assumes 100 characters is enough

That should be

snprintf(buffer, 100, "%s %d", title, value); // this assumes 100 characters is enough

Lost User

Thank you, well spotted.

Vaclav_

Thanks, so I goofed by not allocating space / format for the "concatenated " message. Since this is just basic "title value " function the new buffer should not be fixed / defined. I am planning to pass more values with same title later. But it is a good practice not to allocate more than necessary, right? Thanks again.

Lost User

Yes, otherwise you tend to duplicate effort and waste space. In fact my code relies on the total generated string being less than 100 characters, and could fail for longer titles. A better way of doing it would be to measure the fields that are to be entered to ensure you have enough space. Something like:

char * CLASS_LCM1602::BuildMessage(char *title, int value)
{
// Get the length of the source string plus a space and space for the integer value
// 2147483647 is the maximum value for an integer, so add a sign to get the max length
// add 1 at the end for the trailing null character
int bufferSize = strlen(title) + 1 + strlen("-2147483647") + 1; // actual maximum space required
// allocate a new buffer for the return message
char* buffer = (char*)malloc(bufferSize);
snprintf(buffer, bufferSize, "%s %d", title, value); // this assumes 100 characters is enough

return buffer;

}

Joe Woodbury

Note that title is a static string, not a buffer; you cannot safely append to it. Your prototype should be more like:

BuildMessage(const char* pTitle, int value, char* pDstBuffer, size_t dstBufferLen);

Also note that you are doing an snprintf, but aren't using strncat.

Vaclav_

When I started I actually had two function steps - convert value int to char* then strcat the title and value. It did not work so I inserted strcpy, but forgot the build resulting buffer.

snprintf

was just to do conversion. Now it does it all in few lines of code. Thanks to the forum. Appreciate it.

leon de boer

Personally code in this form I would not allow in our code base as it will lead to bleeding memory when used by others. It is not clear that the function allocates memory, to anyone other than yourself and not being aware the function setups to use the char* pointer return and lose it ... take this basic code

CLASS_LCM1602 myCLASS_LCM1602 = .. some initialization;

/* now use your function */
std::cout << myCLASS_LCM1602.BuildMessage("Demo my Title", 10) << "\n";

That bleeds memory and the whole setup of the function encourages it because people will be tempted to feed the return char* value into functions. For my 2 cents I would force you to use this interface for what you wanted to do .. Joe below has made the same comment.

int CLASS_LCM1602::BuildMessage(const char *title, int value, char* buffer, int bufSize)

It is much cleaner in that they pass you a buffer to fill and it's size and you return how many characters you put in the buffer. Both you and they can be safe about handling the string becasue all sizes are known both directions. If they want to allocate a buffer they can but they then know they have responsibility to dispose it. It is also more flexible because you can use it on temporary stack allocated buffers .. something like this is a perfectly valid temp buffer with no allocate required.

char tempBuf[256];

As an example the actual Windows framework MFC is copying does exactly that :-) GetWindowTextA function | Microsoft Docs[^]

In vino veritas