[olsr-dev] [PATCH] Improvements in dot_draw plugin
Andreas Tønnesen
(spam-protected)
Mon Nov 7 07:14:23 CET 2005
Hi Bernd,
Thanks, patches are always appreciated :-)
But if you can make them against the current CVS version you'll make
life easier for everybody and you make sure you're not fixing something
that is already fixed.
Regarding the sprintf issue I agree that it is bad, but in this code the
string arguments passed to the %s format is always the result for the
olsr_ip_to_string fuction which I regard to be a "controlled" result.
In the core olsrd code there is (AFAIK) no use of potentially dangerous
string functions, but in plugins such as this I don't think this is a
big problem. But if you'd like to clean it up in a platform independent
way, feel free to do so :-)
- andreas
Bernd Petrovitsch wrote:
> The attached patch removes lots of duplicated stuff by adding a
> ipc_send_str() function.
> Furtehr a few typos are corrected and a function and several variables
> were made "static".
> It is against 0.4.9.
>
> The open questions are:
> - Do you actually appreciate such patches?
> And in that format?
> - There severe problems there also: The use of sprintf() is dangerous
> since the function doesn't know how long the buffer.
> The obvious fix is to replace it (unconditionally) with the - now
> common on glibc systems - snprintf() (which I could do). The question
> is if non-Linux-OSes have a problem with such a fix.
> I'm also not a friend of strcpy()/strcat() for the same reason.
> And BTW strncpy()/strncat() have the problem that they do not
> guarantee a terminating \0 character in the string.
> - Do you want more such "cleanup" patches?
> And can they be done so that *BSD and Win* do not suffer from
> Linux-isms I'm probably introducing?
>
> Bernd
>
>
> ------------------------------------------------------------------------
>
> diff --exclude='*.o' --exclude='*~' --exclude='#*#' --exclude='.#*' -urN olsrd-0.4.9/lib/dot_draw/src/olsrd_dot_draw.c olsrd-0.4.9-patched/lib/dot_draw/src/olsrd_dot_draw.c
> --- olsrd-0.4.9/lib/dot_draw/src/olsrd_dot_draw.c 2005-02-20 16:51:15.000000000 +0100
> +++ olsrd-0.4.9-patched/lib/dot_draw/src/olsrd_dot_draw.c 2005-11-06 21:44:59.482979669 +0100
> @@ -54,10 +54,11 @@
> #define close(x) closesocket(x)
> #endif
>
> -int ipc_socket;
> -int ipc_open;
> -int ipc_connection;
> -int ipc_socket_up;
> +static int ipc_socket;
> +static int ipc_open;
> +static int ipc_connection;
> +static int ipc_socket_up;
> +
>
> static double
> calc_etx(double, double);
> @@ -65,6 +66,11 @@
> static void inline
> ipc_print_neigh_link(struct neighbor_entry *);
>
> +static int
> +ipc_send(char *data, int size);
> +
> +static int
> +ipc_send_str(char *data);
>
>
>
> @@ -72,14 +78,13 @@
> ipc_print_neigh_link(struct neighbor_entry *neighbor)
> {
> char buf[256];
> - int len;
> char* adr;
> double etx=0.0;
> char* style = "solid";
> struct link_entry* link;
> adr = olsr_ip_to_string(main_addr);
> - len = sprintf( buf, "\"%s\" -> ", adr );
> - ipc_send(buf, len);
> + sprintf( buf, "\"%s\" -> ", adr );
> + ipc_send_str(buf);
>
> adr = olsr_ip_to_string(&neighbor->neighbor_main_addr);
>
> @@ -99,12 +104,12 @@
> }
> }
>
> - len = sprintf( buf, "\"%s\"[label=\"%.2f\", style=%s];\n", adr, etx, style );
> - ipc_send(buf, len);
> + sprintf( buf, "\"%s\"[label=\"%.2f\", style=%s];\n", adr, etx, style );
> + ipc_send_str(buf);
>
> if (neighbor->is_mpr) {
> - len = sprintf( buf, "\"%s\"[shape=box];\n", adr );
> - ipc_send(buf, len);
> + sprintf( buf, "\"%s\"[shape=box];\n", adr );
> + ipc_send_str(buf);
> }
> }
>
> @@ -232,7 +237,7 @@
>
>
>
> -/* Mulitpurpose funtion */
> +/* Multipurpose function */
> int
> plugin_io(int cmd, void *data, size_t size)
> {
> @@ -271,7 +276,7 @@
> {
> /* Print tables to IPC socket */
>
> - ipc_send("digraph topology\n{\n", strlen("digraph topology\n{\n"));
> + ipc_send_str("digraph topology\n{\n");
>
> /* Neighbors */
> for(index=0;index<HASHSIZE;index++)
> @@ -326,7 +331,7 @@
> }
>
>
> - ipc_send("}\n\n", strlen("}\n\n"));
> + ipc_send_str("}\n\n");
>
> res = 1;
> }
> @@ -353,17 +358,16 @@
> ipc_print_tc_link(struct tc_entry *entry, struct topo_dst *dst_entry)
> {
> char buf[256];
> - int len;
> char* adr;
> double etx = calc_etx( dst_entry->link_quality, dst_entry->inverse_link_quality );
>
> adr = olsr_ip_to_string(&entry->T_last_addr);
> - len = sprintf( buf, "\"%s\" -> ", adr );
> - ipc_send(buf, len);
> + sprintf( buf, "\"%s\" -> ", adr );
> + ipc_send_str(buf);
>
> adr = olsr_ip_to_string(&dst_entry->T_dest_addr);
> - len = sprintf( buf, "\"%s\"[label=\"%.2f\"];\n", adr, etx );
> - ipc_send(buf, len);
> + sprintf( buf, "\"%s\"[label=\"%.2f\"];\n", adr, etx );
> + ipc_send_str(buf);
> }
>
> static void inline
> @@ -372,28 +376,36 @@
> char *adr;
>
> adr = olsr_ip_to_string(gw);
> - ipc_send("\"", 1);
> - ipc_send(adr, strlen(adr));
> - ipc_send("\" -> \"", strlen("\" -> \""));
> + ipc_send_str("\"");
> + ipc_send_str(adr);
> + ipc_send_str("\" -> \"");
> adr = olsr_ip_to_string(net);
> - ipc_send(adr, strlen(adr));
> - ipc_send("/", 1);
> + ipc_send_str(adr);
> + ipc_send_str("/");
> adr = olsr_netmask_to_string(mask);
> - ipc_send(adr, strlen(adr));
> - ipc_send("\"[label=\"HNA\"];\n", strlen("\"[label=\"HNA\"];\n"));
> - ipc_send("\"", 1);
> + ipc_send_str(adr);
> + ipc_send_str("\"[label=\"HNA\"];\n");
> + ipc_send_str("\"");
> adr = olsr_ip_to_string(net);
> - ipc_send(adr, strlen(adr));
> - ipc_send("/", 1);
> + ipc_send_str(adr);
> + ipc_send_str("/");
> adr = olsr_netmask_to_string(mask);
> - ipc_send(adr, strlen(adr));
> - ipc_send("\"", 1);
> - ipc_send("[shape=diamond];\n", strlen("[shape=diamond];\n"));
> + ipc_send_str(adr);
> + ipc_send_str("\"");
> + ipc_send_str("[shape=diamond];\n");
> }
>
>
> +static int
> +ipc_send_str(char *data)
> +{
> + if(!ipc_open)
> + return 0;
>
> -int
> + return ipc_send(data, strlen(data));
> +}
> +
> +static int
> ipc_send(char *data, int size)
> {
> if(!ipc_open)
> @@ -469,14 +481,13 @@
> {
> in.s_addr = mask->v4;
> ret = inet_ntoa(in);
> - return ret;
> -
> }
> else
> {
> /* IPv6 */
> + static char netmask[5];
> sprintf(netmask, "%d", mask->v6);
> - return netmask;
> + ret = netmask;
> }
>
> return ret;
> diff --exclude='*.o' --exclude='*~' --exclude='#*#' --exclude='.#*' -urN olsrd-0.4.9/lib/dot_draw/src/olsrd_dot_draw.h olsrd-0.4.9-patched/lib/dot_draw/src/olsrd_dot_draw.h
> --- olsrd-0.4.9/lib/dot_draw/src/olsrd_dot_draw.h 2005-02-20 16:51:15.000000000 +0100
> +++ olsrd-0.4.9-patched/lib/dot_draw/src/olsrd_dot_draw.h 2005-11-06 21:43:49.931510460 +0100
> @@ -50,9 +50,7 @@
> #include "olsrd_plugin.h"
>
>
> -char netmask[5];
> -
> -/* Event function to register with the sceduler */
> +/* Event function to register with the scheduler */
> int
> pcf_event(int, int, int);
>
> @@ -68,9 +66,6 @@
> static void inline
> ipc_print_net(union olsr_ip_addr *, union olsr_ip_addr *, union hna_netmask *);
>
> -int
> -ipc_send(char *, int);
> -
> char *
> olsr_ip_to_string(union olsr_ip_addr *);
>
>
>
> ------------------------------------------------------------------------
>
> _______________________________________________
> olsr-dev mailing list
> (spam-protected)
> https://www.olsr.org/mailman/listinfo/olsr-dev
--
Andreas Tønnesen
http://www.olsr.org
http://www.tønnesen.org
More information about the Olsr-dev
mailing list