2010-05-14 9 views
6

que tienen una función irc_sendline que se puede llamar como printf puedevariable C argumento refactorización

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

Funciona perfectamente, pero no estoy feliz con su implementación:

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; 
} 

Ves, Estoy usando 2 búferes "temporales" aquí, porque primero tengo que copiar el mensaje original de los argumentos de la función en un búfer temporal para agregarle "\ r \ n" y luego copiar ese búfer temporal en otro temporal buffe r para hacer el formateo real con los argumentos proporcionados desde la llamada a la función, y solo ENTONCES Puedo enviar el material en camino.

¿Cómo podría hacer que este limpiador, mejor?


Gracias por toda la entrada aquí, pensé que mi único problema era el desorden allí, pero era en realidad una bomba de relojería! Mi nueva función es el siguiente:

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; 
} 

Si pudiera, me gustaría aceptar múltiples respuestas aquí, pero meh.

+0

Su código se rompe debido a la incorrecta utilización de 'strncpy' (véase mi respuesta). – AnT

+0

Su uso de 'strncat' también está roto. 'strncat' no hace lo que crees que hace. – AnT

Respuesta

5

Primero use vsnprintf para formatear los datos, luego agregue el "\ r \ n" al resultado de eso. Alternativamente, simplemente use una segunda llamada al send para enviar el mensaje "\ r \ n".

1

Desde el \r\n va a terminar en el extremo de la cadena con formato, por qué no copiar después:

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

Tenga en cuenta que también fija los argumentos a strncat.

0

A menos que desee utilizar msg para strcat (inseguro y malo porque no conoce el tamaño de la cadena), creo que tendrá que vivir con 2 almacenamientos intermedios.

Como comentario adicional, consideraría strncpy (..., BUFSIZE-2) para que \ r \ n siempre entre en tus mensajes y, por lo tanto, las cadenas siempre se envuelvan.

+0

+1 para 'BUFSIZE-2'. –

+0

¡Gracias por señalar lo de BUFSIZE! No me di cuenta de eso en absoluto, podría haberme arruinado. – LukeN

+0

Aún se está olvidando que 'stncpy' no agrega el terminador nulo a la cadena si el buffer es demasiado corto. 'BUFSIZE-2' no arreglará eso. Su código se bloqueará cada vez que el final del búfer sea golpeado por la cadena de formato. – AnT

3

En primer lugar, si está interesado en "limpiador", deje de usar strncpy. A pesar del nombre engañoso (y contrario a la creencia popular), esta no es una función de copia de cadenas de longitud limitada. Es seguro decir que strncpy es una función que prácticamente no tiene ningún uso práctico en la actualidad. Cuando vea strncpy utilizado en el código, el "limpiador" queda inmediatamente fuera de toda duda (aparte del conjunto cada vez más estrecho de los estuches strncpy estaba destinado a ser utilizado).

De hecho, su código se rompe específicamente debido a que utilizó strncpy: si la longitud de la cadena de formato es mayor que la longitud de la memoria intermedia, strncpyno añade la terminación carácter nulo al resultado, lo que significa que la la llamada siguiente strncat se bloqueará (en el mejor de los casos).

La función de copia de longitud limitada no existe en la biblioteca estándar, pero a menudo la proporciona la implementación bajo el nombre strlcpy. Si la implementación que está utilizando no proporciona uno, escriba uno usted mismo y úselo.

Su código también se rompe debido al uso incorrecto de strncat: strncat no toma la longitud del búfer completo como último argumento. En cambio, strncat espera la longitud del resto disponible del búfer. Por lo tanto, si desea utilizar strncat, primero debe calcular cuánto espacio quedaba al final del almacenamiento intermedio después de la copia anterior. De nuevo, aunque strncat es más útil que strncpy, es mejor utilizar la función no estándar (pero a menudo proporcionada por la implementación) strlcat, que en realidad funciona de la forma en que creía que strncat funcionó.

En segundo lugar, en lugar de agregar la pieza \r\n por adelantado, ¿por qué no lo haces después? Use el hecho de que vsnprintf devuelve la cantidad de caracteres escritos en el búfer de salida y simplemente anexa los caracteres \r, \n y \0 al final después de que vsnprintf haya terminado de funcionar. No es necesario que use strncat para tal fin. Simplemente escriba los caracteres en el búfer directamente, asegurándose, por supuesto, de que no cruce el límite.

+0

Gracias por la advertencia (3 de ellos en realidad :), actualmente estoy buscando este tema, y ​​parece que tengo que escribir mi propio strlcpy, porque Linux no lo proporciona. En el tema principal, ¿está roto porque no agregará \ NUL cuando la cadena objetivo es más larga/tan larga como la cadena de destino? – LukeN

+0

En primer lugar, sí, no agrega '\ 0' si la cadena es más larga. En segundo lugar, llena el resto del buffer con ceros si la cadena es más corta, lo cual es completamente innecesario. Una vez más, 'strncpy' no es una función que se creó para cadenas terminadas en nulo. Es una función que se creó para un propósito completamente diferente: para admitir las llamadas cadenas de "ancho fijo" en alguna versión anterior del sistema de archivos Unix. Puede leer más sobre esto aquí: http://stackoverflow.com/questions/1453876/why-does-strncpy-not-null-terminate – AnT

+0

Gracias por aclarar esto, ahora recuerdo por qué terminé explícitamente cero mis cadenas en algunos otros programas que usan strncpy. – LukeN

0

Un problema importante con su código - vsnprintf devuelve el número de caracteres que se colocarían en el búfer si fuera infinitamente grande, que puede ser mayor que BUFSIZE si el búfer no es lo suficientemente grande. Entonces, si tiene un mensaje que se desborda, terminará enviando basura al azar después del final de su búfer. Es necesario añadir una línea

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

después de la vprintf si desea truncar en realidad el mensaje

Cuestiones relacionadas