make_single_line_string maximum length

[hreintke]

LS,

In utils/simstring cc the function make_single_line string is defined as below

Code Select Expand


// copies a n into a single line and maximum 128 characters
// @author prissi
char *make_single_line_string(const char *in,int number_of_lines)
{
static char buf[64];
int pos;

// skip leading whitespaces
while(*in=='\n'  ||  *in==' ') {
in++;
}
// start copying
for(pos=0;  pos<62  &&  *in!=0  &&  number_of_lines>0;  ) {
if((unsigned)(*in)>' ') {
buf[pos++] = *in++;


In the comment it states maximum 128 chars, in the code it it limited to 64 characters.
I would like to use 128 of even 256 as maximum.

Looked for the usage and did not see issues updating the code to a higher value.

Does anyone know whether the "downsize" to 64 was intential and if yes for what reason ?

Kind regards,

Herman

[Ters]

Changing the fixed limit means that callers expecting to get a maximum of 64/128 characters will get more than they ask for, which is bad. Making the limit dynamic could have some impact on performance. Is this called from something that might be performance critical?

[hreintke]

LS,

Don't think it is performance critical.

It is currently used in
- simcity when creating the message for a new attraction
- simhalt when finding a new name for a new halt
- ai_passenger when creating a message when adding a service.

Because of that limited use I asked the question whether I maybe overlooked something.
It's always better to use (updated) exsisting fuctionality then creating another almost identical support routine.

I noticed it with my experimenting getting a webserver included in simutrans and returning simutrans messages to the client. In the "msg" part of the message_t a lot of newlines are in.

Herman

[Ters]

I think the best solution is to make the function take the destination char array and it's length as parameters. The current implementation isn't thread safe, which wasn't a problem when Simutrans didn't use threads, but there has recently been done some work on that. And document it properly. The number_of_lines parameter isn't obvious.

[prissi]

simticker.cc has a similar function (for 256 chars) Maybe it is time to merge those and also give a destination length parameter ...

[hreintke]

Ters,

What makes the make_single_line_string function not thread safe ? Is is the use of static char buf[64] as intermediate ?

@Prissi : Do you want to keep the "int number_of_lines" as an additional parameter ?

Herman

[Ters]

What makes the make_single_line_string function not thread safe ? Is is the use of static char buf[64] as intermediate ?

Yes. It is also the return value, so even within the same thread you can run into trouble if the function is called again before the result is copied somewhere else.