Hacker News new | ask | show | jobs
by ams6110 4250 days ago
Those would be "safe" (assuming that settings.getIMEI() is completely under your control, everything else is string literals) but yeah snprintf seems way better here (though it's been well over 20 years since I wrote any significant C code.
2 comments

Possibly safe but definitely inefficient, since it has to find the end of the string to know where the destination pointer starts. The right way is to keep a pointer to the end.

(Or since they are already using std::string in other places, maybe just do that everywhere, I'm sure it makes better choices than they did here.)

The pointer cast thing is glaring. Why not simply declare the buffer as a char array and be done with it, instead of casting at every use? IMO over-use of pointer casts is a clear sign someone is lost in the language, your goal should be to reduce them.

Yeah agree, to me casts like that are a smell that someone is trying to squash compiler complaints rather than understanding them. It also has every appearance of "copy/paste" code writing.
Since these are all string literals you really don't need any concatenation function at all except to concatenate with the output of getIMEI().

  char *a = "Hello " "world!";
Works just fine.

Edit to add: You can really see the difference in code between someone coming to C/C++ from a high level language and someone who learned assembly first, where a list of literals is a common idiom. The original style is not functionally wrong, but it does look like Java :-)

Also: DON'T post your potentially insecure string handling code on the Internet; are you crazy?