Я пытаюсь добавить две строки вместе, используя memcpy. Первый memcpy содержит данные, которые мне нужны. Однако второй не добавляется. Есть идеи, почему?
if (strlen(g->db_cmd) < MAX_DB_CMDS )
{
memcpy(&g->db_cmd[strlen(g->db_cmd)],l->db.param_value.val,strlen(l->db.param_value.val));
memcpy(&g->db_cmd[strlen(g->db_cmd)],l->del_const,strlen(l->del_const));
g->cmd_ctr++;
}
size_t len = strlen(l->db.param_value.val);
memcpy(g->db_cmd, l->db.param_value.val, len);
memcpy(g->db_cmd + len, l->del_const, strlen(l->del_cost)+1);
Это дает вам следующее:
strlen. Каждый из них должен проходить через строку, поэтому рекомендуется свести к минимуму эти вызовы.memcpy должен фактически добавляться, а не заменяться. Поэтому первый аргумент должен отличаться от предыдущего вызова.+1 в третьем аргументе второго memcpy. Это для терминатора NUL.Я тоже не уверен, что ваше утверждение if имеет смысл. Возможно, более разумным было бы убедиться, что в g->db_cmd достаточно места для того, что вы собираетесь копировать. Вы могли бы сделать это либо с помощью sizeof (если db_cmd — это массив символов), либо отслеживая, насколько велики ваши распределения кучи (если db_cmd было получено с помощью malloc). Так что, возможно, это будет иметь смысл, как:
size_t param_value_len = strlen(l->db.param_value.val),
del_const_len = strlen(l->del_const);
// Assumption is that db_cmd is a char array and hence sizeof(db_cmd) makes sense.
// If db_cmd is a heap allocation, replace the sizeof() with how many bytes you
// asked malloc for.
//
if (param_value_len + del_const_len < sizeof(g->db_cmd))
{
memcpy(g->db_cmd, l->db.param_value.val, param_value_len);
memcpy(g->db_cmd + param_value_len, l->del_const, del_const_len + 1);
}
else
{
// TODO: your buffer is not big enough. handle that.
}
Вы не копируете нулевой терминатор, вы копируете только необработанные строковые данные. Это оставляет вашу строку не завершающейся нулем, что может вызвать всевозможные проблемы. Вы также не проверяете, достаточно ли места в буфере, что может привести к уязвимости переполнения буфера< /а>.
Чтобы убедиться, что вы скопировали нулевой терминатор, просто добавьте 1 к количеству копируемых байтов — скопируйте strlen(l->db.param_value.val) + 1 байт.
Одна из возможных проблем заключается в том, что ваш первый вызов memcpy() не обязательно приведет к строке с завершающим нулем, поскольку вы не копируете терминатор '\0' из l->db.param_value.val:
Поэтому, когда strlen(g->db_cmd) вызывается во втором вызове memcpy(), он может возвращать что-то совершенно фиктивное. Является ли это проблемой, зависит от того, инициализирован ли буфер g->db_cmd нулями заранее или нет.
Почему бы не использовать strcat(), который был создан именно для того, что вы пытаетесь сделать с memcpy()?
if (strlen(g->db_cmd) < MAX_DB_CMDS )
{
strcat( g->db_cmd, l->db.param_value.val);
strcat( g->db_cmd, l->del_const);
g->cmd_ctr++;
}
Это будет иметь то преимущество, что кому-то будет легче читать. Вы можете подумать, что это будет менее производительно, но я так не думаю, поскольку вы явно делаете кучу strlen() вызовов. В любом случае, я бы сначала сосредоточился на том, чтобы все было правильно, а потом уже беспокоился о производительности. Неправильный код настолько неоптимизирован, насколько это возможно — сделайте его правильно, прежде чем делать быстро. На самом деле, мой следующий шаг будет заключаться не в улучшении кода с точки зрения производительности, а в том, чтобы улучшить код, чтобы снизить вероятность переполнения буфера (вероятно, я бы переключился на использование чего-то вроде strlcat() вместо strcat()).
Например, если g->db_cmd является массивом символов (а не указателем), результат может выглядеть так:
size_t orig_len = strlen(g->db_cmd);
size_t result = strlcat( g->db_cmd, l->db.param_value.val, sizeof(g->db_cmd));
result = strlcat( g->db_cmd, l->del_const, sizeof(g->db_cmd));
g->cmd_ctr++;
if (result >= sizeof(g->db_cmd)) {
// the new stuff didn't fit, 'roll back' to what we started with
g->db_cmd[orig_len] = '\0';
g->cmd_ctr--;
}
Если strlcat() не является частью вашей платформы, его довольно легко найти в сети. Если вы используете MSVC, вместо нее можно использовать функцию strcat_s() (но учтите, что она не эквивалентна strlcat() — вам придется изменить способ проверки и обработки результатов вызова strcat_s()).
