Объединить с memcpy - PullRequest
Новая
       4

Объединить с memcpy

4 голосов
/ 03 августа 2011

Я пытаюсь добавить две строки вместе, используя 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++;
}

Ответы [ 3 ]

7 голосов
/ 03 августа 2011 
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. Каждый из них должен проходить строку, поэтому рекомендуется минимизировать эти вызовы.
  • 2-й 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 голос
/ 03 августа 2011

Одной из возможных проблем является то, что ваш первый memcpy() вызов не обязательно приведет к завершению строкой с нулевым символом, поскольку вы не копируете терминатор '\ 0' из l->db.param_value.val:

Таким образом, когда во втором вызове memcpy() вызывается strlen(g->db_cmd), он может возвращать что-то полностью поддельное. Является ли это проблемой, зависит от того, инициализирован ли буфер 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()).

1 голос
/ 03 августа 2011

Вы не копируете нулевой терминатор, вы только копируете необработанные строковые данные.Это приводит к тому, что ваша строка не заканчивается нулем, что может вызвать всевозможные проблемы.Вы также не проверяете, достаточно ли места в вашем буфере, что может привести к уязвимости переполнения буфера .

Чтобы убедиться, что вы скопировали нулевой терминатор, просто добавьте 1 кколичество копируемых байтов - скопируйте strlen(l->db.param_value.val) + 1 байтов.

...