Re: [KJ] [patch] net/tipc: sprintf/strcpy conversion

[Hagen Paul Pfeifer]

* Alexey Dobriyan | 2006-11-03 03:09:05 [+0300]:

On Wed, Nov 01, 2006 at 03:06:24PM +0100, Florian Westphal wrote:
 convert sprintf(a,b) to strcpy(a,b). Make tipc_bclink_name[] const.

Ahhh, I missed the start of threads.

Patch is useless because it changes one unbounded string function into
another unbounded string function.

The discussion in this thread is really back-breaking!

1. To make tipc_bclink_name const there is absolutly no objection
2. Replace sprintf with strcpy

  a) First of all: If you _copy_ a string then use also strCPY()
 Thats a question of good style!

  b) If the compiler is smart enough, he realize that you want to copy 
 a string and replace the sprintf call with a
pushl   %ebx
callstrcpy
 Surprise - Surprise!

 Assumed you use gcc with -Os or -O2! Don't know how icc handle this 
 case. If you compile without optimization you save at least a
 repz movsb %ds:(%esi),%es:(%edi) instruction.

   c) Last but not least I read all the time this patch doesn't introduce
  bounds-checking. This isn't a argument because the author is aware of
  the destination length of the buffer. BTW: grep for (sprintf|strcpy) in
  /usr/src/linux and be surprised how unsecure the kernel is (thats
  ironical).

This patch is 100% OK!

HGN



-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [KJ] [patch] net/tipc: sprintf/strcpy conversion

[Alexey Dobriyan]

On Wed, Nov 01, 2006 at 03:06:24PM +0100, Florian Westphal wrote:
 convert sprintf(a,b) to strcpy(a,b). Make tipc_bclink_name[] const.

Ahhh, I missed the start of threads.

Patch is useless because it changes one unbounded string function into
another unbounded string function.

 --- a/net/tipc/bcast.c
 +++ b/net/tipc/bcast.c
 @@ -790,7 +790,7 @@ int tipc_bclink_init(void)
   INIT_LIST_HEAD(bcbearer-bearer.cong_links);
   bcbearer-bearer.media = bcbearer-media;
   bcbearer-media.send_msg = tipc_bcbearer_send;
 - sprintf(bcbearer-media.name, tipc-multicast);
 + strcpy(bcbearer-media.name, tipc-multicast);

   bcl = bclink-link;
   memset(bclink, 0, sizeof(struct bclink));
 @@ -802,7 +802,7 @@ int tipc_bclink_init(void)
   tipc_link_set_queue_limits(bcl, BCLINK_WIN_DEFAULT);
   bcl-b_ptr = bcbearer-bearer;
   bcl-state = WORKING_WORKING;
 - sprintf(bcl-name, tipc_bclink_name);
 + strcpy(bcl-name, tipc_bclink_name);

   if (BCLINK_LOG_BUF_SIZE) {
   char *pb = kmalloc(BCLINK_LOG_BUF_SIZE, GFP_ATOMIC);
 --- a/net/tipc/node.c
 +++ b/net/tipc/node.c
 @@ -667,7 +667,7 @@ struct sk_buff *tipc_node_get_links(cons
  link_info.dest = tipc_own_addr  0xf00;
   link_info.dest = htonl(link_info.dest);
  link_info.up = htonl(1);
 -sprintf(link_info.str, tipc_bclink_name);
 +strcpy(link_info.str, tipc_bclink_name);

-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [KJ] [patch] net/tipc: sprintf/strcpy conversion

[David Miller]

From: walter harms [EMAIL PROTECTED]
Date: Wed, 01 Nov 2006 22:38:26 +0100

 These line
 + strcpy(bcbearer-media.name, tipc-multicast);
 
 i gues that means tipc_bclink_name ?

Why?  The original code used tipc-multicast which is a
different string than tipc_bclink_name which is multicast-link.

  -   sprintf(bcbearer-media.name, tipc-multicast);
  +   strcpy(bcbearer-media.name, tipc-multicast);

If you are arguing that it should be changed, that's a different
changeset to discuss.
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [KJ] [patch] net/tipc: sprintf/strcpy conversion

[walter harms]

David Miller wrote:
 From: walter harms [EMAIL PROTECTED]
 Date: Wed, 01 Nov 2006 22:38:26 +0100
 
 These line
 +strcpy(bcbearer-media.name, tipc-multicast);

 i gues that means tipc_bclink_name ?

mea culpa, i should not write mail when tired.


 
 Why?  The original code used tipc-multicast which is a
 different string than tipc_bclink_name which is multicast-link.
 
 -   sprintf(bcbearer-media.name, tipc-multicast);
 +   strcpy(bcbearer-media.name, tipc-multicast);
 
 If you are arguing that it should be changed, that's a different
 changeset to discuss.
 
 
 
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html