Re: [patch] support xclipboard

[Jindrich Novy <jnovy redhat com>]

Hi Vadim,

On Thu, 2006-03-16 at 12:09 +0200, vadim wrote:
> Patch for support XCLIPBOARD from mc.
> It can work in 2 way:
> - using Xlib if mc compile with-x
> - using external program if mc compile without-x
> External program is xclip ( http://people.debian.org/~kims/xclip/ ) and qtclipb ( attached file or http://www.klv.lg.ua/~vadim/ ).
> qtclipb be writed for work with KDE since xclip can not insert into XCLIPBOARD under KDE.

I like the idea, but I have a few comments related to the patch:

1)
sprintf(mc_cl_file, "%s/.mc/%s", home_dir, XCLIPB_FILE);

Please use snprintf instead to prevent buffer overflows. If the patch is
commited as-is the long home_dir may cause overwriting of en_xcl what
causes unintentional change of functionality of your patch because
mc_cl_file is declared like: 

+static char mc_cl_file [256];
+//static FILE *fl = NULL;
+static int en_xcl = 0; /* 1 -> enable xclipboard ,  -1 -> found kde */

2)
+    //if ( start >= finish ) {
+       /* for ( ; i < finish ; i++ ) {
+           buff[i - start] = edit_get_byte (edit, i);
+       }*/
+    buff = edit_get_block (edit, start, finish, &len);
+    //}

Please avoid putting a commented out code in the patch, this only
confuses the reviewer as well as these useless hunks:

@@ -2366,7 +2438,7 @@

     exp = old ? old : "";

-    exp = input_dialog (_(" Run Sort "),
+    exp = input_dialog (_(" Run Sort "),
     _(" Enter sort options (see manpage) separated by whitespace: "),
exp);

     if (!exp)

Thanks,
Jindrich