Re[2]: [PATCH] edit.c, init_dynamic_edit_buffers (was Re[2]: CR/LF translation)

[Pavel Roskin <proski gnu org>]

Hello!

> Should be fixed now.

Yes, everything compiles now.  It even work for me.

As we discussed before, I would prever the editor not to rely on data from 
stat(), but read the file until EOF.  Please consider if it can be part of 
your patch, or it's better to fix separately.

I don't know much about the internals of the editor, and I don't have time
to learn it today, but let me go through your code and check some little
details.

    int crlf_maybe = FALSE;    /* See below. */ 

That's hard to read.  I would prefer an enum with 3 values: CRLF_TRUE, 
CRLF_FALSE and CRLF_MAYBE.  gdb understands enums, so it will be easier to 
debug.

    size_t nblkout = 0;                /* The number of output blockd 

I think you mean "blocks".

                       /* We can use memcpy () here. */ 
                       memcpy (putp, getp, nbytes); 

The comment is unclear.  Actually, we should use memcpy() instead of 
strcpy(), because we want to support binary zeroes.

                       /* Ugly! */ 
                       size_t nbytes_sav = nbytes; 

Only you know why it's ugly.  Use comments meaningful to others or no 
comments.

           if (mc_read (file, (char *) edit->buffers2[j], nread) != 
                       nread) { 
               /* TODO: Add some message here - I dunno how :( */ 

mc_read() is usually read(), i.e. low-level read().  Unlike fread() which 
is buffered in libc, it's common for read() to return less bytes than 
requested.  From info libc:

     The return value is the number of bytes actually read.  This might
     be less than SIZE; for example, if there aren't that many bytes
     left in the file or if there aren't that many bytes immediately
     available.  The exact behavior depends on what kind of file it is.
     Note that reading less than SIZE bytes is not an error.

Looking at the size of the new code, I start wondering if it's really
worth the trouble to write all that just to skip CR characters from input.  
Do you expect any other positive effects from your patch other than CRLF
suppport?

> Btw there are some unpleasent bugs in the editor - and I dont really
> think that it can open gigabyte files. Actually it is limited to 65Mb
> from what I can see.

That's right.  But I think it should be possible to make it work with
large files, even with files over 4G on 32-bit systems.  That's because we
have two-level addressing - buffer number and byte number in the buffer.

-- 
Regards,
Pavel Roskin