Рефакторинг аргумента переменной C - PullRequest
6 голосов
/ 14 мая 2010

У меня есть функция irc_sendline, которая может быть вызвана как printf can

irc_sendline(s, "A strange game.\nThe only %s is not to play.", "winning move");

Отлично работает, но я не доволен его реализацией:

int irc_sendline(irc *iobj, char *msg, ...)
{
   char tmp_msg[BUFSIZE], fmsg[BUFSIZE];
   va_list args;
   int len;

   va_start(args, msg);

   strncpy(tmp_msg, msg, BUFSIZE);
   strncat(tmp_msg, "\r\n", BUFSIZE);

   len = vsnprintf(fmsg, BUFSIZE, tmp_msg, args);
   len = send(iobj->fd, fmsg, len, 0);

   return len;
}

Видите ли, я здесь использую 2 «временных» буфера, потому что сначала мне нужно скопировать исходное сообщение из аргументов функции во временный буфер, чтобы добавить к нему «\ r \ n», а затем скопировать этот временный Буфер для другого временного буфера для выполнения фактического форматирования с аргументами, предоставленными из вызова функции, и только THEN Я могу отправить материал в его пути.

Как я могу сделать это чище, лучше?


Спасибо за весь вклад, я думал, что моей единственной проблемой был беспорядок там, но на самом деле это была бомба замедленного действия! Моя новая функция выглядит так:

int irc_sendline(irc *iobj, char *msg, ...)
{
   char buffer[BUFSIZE];
   va_list args;
   int res_str_len;
   int sent;

   va_start(args, msg);

   res_str_len = vsnprintf(buffer, BUFSIZE, msg, args);

   sent =  send(iobj->fd, buffer, res_str_len, 0);
   sent += send(iobj->fd, "\r\n", 2, 0);

   return sent;
}

Если бы я мог, я бы принял несколько ответов здесь, но ме.

Ответы [ 5 ]

5 голосов
/ 14 мая 2010

Сначала используйте vsnprintf для форматирования данных, затем добавьте «\ r \ n» к результату из этого. В качестве альтернативы просто используйте второй вызов send, чтобы отправить "\ r \ n".

3 голосов
/ 14 мая 2010

Во-первых, если вас интересует «очиститель», прекратите использовать strncpy. Несмотря на вводящее в заблуждение название (и вопреки распространенному мнению), это не функция копирования строк ограниченной длины. Можно с уверенностью сказать, что strncpy - это функция, которая сегодня практически не имеет практического применения. Когда вы видите strncpy, использованный в коде, «уборщик» сразу же становится невозможным (за исключением исчезающе узкого набора случаев, для которых strncpy фактически предназначался для использования).

Фактически, ваш код поврежден именно потому, что вы использовали strncpy: если длина строки формата больше, чем длина буфера, strncpy не добавляет завершающий нулевой символ к результату, означающему, что последующий вызов strncat завершится сбоем (в лучшем случае).

Функция копирования ограниченной длины не существует в стандартной библиотеке, но часто предоставляется реализацией под именем strlcpy. Если используемая вами реализация не предоставляет такую ​​возможность - напишите ее самостоятельно и используйте ее.

Ваш код также поврежден из-за неправильного использования strncat: strncat не принимает длину полного буфера в качестве последнего аргумента. Вместо этого strncat ожидает длину доступного остатка буфера. Итак, если вы хотите использовать strncat, вы должны сначала рассчитать, сколько места осталось в конце буфера после предыдущего копирования. Опять же, даже если strncat более полезен, чем strncpy, вам может быть лучше использовать нестандартную (но часто предоставляемую реализацией) функцию strlcat, которая на самом деле работает так, как вы думали, strncat работает.

Во-вторых, вместо того, чтобы добавлять деталь \r\n заранее, почему бы вам не сделать это потом? Используйте тот факт, что vsnprintf возвращает количество символов, записанных в выходной буфер, и просто добавьте символы \r, \n и \0 в конце после окончания работы vsnprintf. Вам не нужно использовать strncat для этой цели. Просто запишите символы непосредственно в буфер, убедившись, конечно, что вы не пересекаете границу.

1 голос
/ 14 мая 2010

Поскольку \r\n заканчивается в конце отформатированной строки, почему бы не скопировать потом:

va_start(args, msg);
len = vsnprintf(fmsg, BUFSIZE, msg, args);
strncat(fmsg, "\r\n", BUFSIZE - strlen(fmsg) - 1);

Обратите внимание, что я также исправил аргументы в strncat.

0 голосов
/ 14 мая 2010

Одна серьезная проблема с вашим кодом - vsnprintf возвращает количество символов, которое будет помещено в буфер, если он будет бесконечно большим, который может быть больше, чем BUFSIZE, если буфер недостаточно велик. Так что, если у вас есть сообщение, которое переполняется, вы в конечном итоге отправите случайный мусор после окончания вашего буфера. Вам необходимо добавить строку

if (res_str_len >= BUFSIZE) res_str_len = BUFSIZE-1

после vprintf, если вы хотите фактически усечь сообщение

0 голосов
/ 14 мая 2010

Если вы не хотели использовать msg для strcat (небезопасно и злобно, потому что вы не знаете размер строки), я думаю, вам придется жить с 2 буферами.

В качестве отступления я бы рассмотрел strncpy (..., BUFSIZE-2), чтобы \ r \ n всегда попадал в ваши сообщения, и поэтому строки всегда переносились.

Добро пожаловать на сайт PullRequest, где вы можете задавать вопросы и получать ответы от других членов сообщества.
...