Re: /dev/clipboard pasting with small read() buffer

[Thomas Wolff <towo at towo dot net>]

On 16.08.2012 14:30, Corinna Vinschen wrote:
> On Aug 16 14:11, Thomas Wolff wrote:
> > Hi Corinna,
> >
> > On 16.08.2012 11:33, Corinna Vinschen wrote:
> > > Hi Thomas,
> > >
> > > thanks for the patch.   I have a few minor nits:
> > >
> > > On Aug 14 22:56, Thomas Wolff wrote:
> > > ... 
> > > > +	  char cprabuf [8 + 1];	/* need this length for surrogates */
> > > > +	  if (len < 8)
> > > > +	    {
> > > > +	      _ptr = cprabuf;
> > > > +	      _len = 8;
> > > > +	    }
> > > 8?  Why 8?  The size appears to be rather artificial.  The code should
> > > use MB_CUR_MAX instead.
> > MB_CUR_MAX does not work because its value is 1 at this point
> So what about MB_LEN_MAX then?  There's no problem using a multiplier,
> but a symbolic constant is always better than a numerical constant.
I've now used _MB_LEN_MAX from newlib.h, rather than MB_LEN_MAX from 
limits.h (note the "_" distinction :) ),
because the latter, by its preceding comment, reserves the option to be 
changed into a dynamic function in the future, which could then possibly 
have the same problems as MB_CUR_MAX.

About the surrogates problem, I think I've found a solution:
I've added an explicit test to avoid processing of split surrogate pairs 
(to that loop...); this seems to work now.

> > > > +	      /* If using read-ahead buffer, copy to class read-ahead buffer
> > > > +	         and deliver first byte. */
> > > > +	      if (_ptr == cprabuf)
> > > > +		{
> > > > +		  puts_readahead (cprabuf, ret);
> > > > +		  * (char *) ptr = get_readahead ();
> > > > +		  ret = 1;
> > > (*) Ok, that works, but wouldn't it be more efficient to do that in
> > > a tiny loop along the lines of
> > >
> > > 		  int x;
> > > 		  ret = 0;
> > >                    while (ret < len && (x = get_readahead ()) >= 0)
> > > 		    ptr++ = x;
> > > 		    ret++;
> > >
> > > ?
> > I can add it if you prefer; I just didn't think it's worth the
> > effort and concerning efficiency, after that prior trial-and-error
> > count-down-loop...
> Yeah, that's a valid point.  But maybe we shouldn't make it slower
> than necessary?  If you have a good idea how to avoid the other
> loop, don't hesitate to submit a patch.
Added the loop to use up the caller's buffer.
About avoiding the trial-and-error loop, I think that would require 
digging into sys_mbstowcs (which doesn't even seem to behave as documented).

------
Thomas

Attachment: clipboard-small-buffer.patch.3
Description: Text document