Larry Jones writes:
>Tony Hoyle writes [using very long lines]:
>> Windows will return the correct length (ie. file size - number
>>of linefeeds) for fstat from text files. On any system which
>>doesn't this is irrelevant, since the return from statbuf is
>>only used once and it will simply alloc/read too many bytes -
>>the read returns the number of actual bytes read, and the function
>>behaves correctly.
>
>Like I said, it's not that simple. There's no guarantee that the file
>size returned by fstat for a text file has any relation whatsoever to
>the number of bytes you can actually read from the file. In particular,
>a record-oriented file system may have to *add* newlines at the record
>boundaries, making the actual size larger, not smaller. Some parts of
>CVS are very careful to do things by the book (i.e., the ANSI/ISO C
>Standard), other parts are cavalier to the point of recklessness. Guess
>which one this is. :-)
Larry,
I have to agree with Tony that there shouldn't be any problems with this
fix.
For example, check out send_modified() function in client.c:
/* Beware: on systems using CRLF line termination conventions,
the read and write functions will convert CRLF to LF, so the
number of characters read is not the same as sb.st_size. Text
files should always be transmitted using the LF convention, so
we don't want to disable this conversion. */
bufsize = sb.st_size;
As the comment states, the conversion should NOT be disabled for text files.
Since the log message text file is transferred as it is, it should also be
treated as a text file.
I'm not aware of any such file system like you mentioned (that would *add*
LF to the lines), but would those be the ones which use the section "#ifdef
BROKEN_READWRITE_CONVERSION" in that same send_modified() function?
That ifdef uses binary mode for "broken" systems, but normal text mode for
others (unless -kb is used). Should that same "#ifdef
BROKEN_READWRITE_CONVERSION" method be used with this log file issue?
Another thing: you said that "the file size returned by *fstat*". If the
*fstat* is the problem, what about if this log file operation would use
CVS_STAT instead (which uses *stat*)? After all, the code already uses
CVS_OPEN right before fstat?
At least the CVS_STAT should work correctly! Otherwise in subr.c the
get_file() function (which is called by RCS_checkin() function, the very
heart of whole CVS!) would be in trouble! It does
if (CVS_STAT (name, &s) < 0)
and would use the value from s.st_size to reallocate the buffer to where it
read the file. The file system, that would LF in the lines, would fail here
anyway ...
During my "research" I also find yet another case: do_editor() function in
logmsg.c uses also CVS_STAT and does
/* On NT, we might read less than st_size bytes, but we won't
read more. So this works. */
*messagep = (char *) xmalloc (post_stbuf.st_size + 1);
Although this is perhaps too "NT" specific comment, wouldn't this part of
the code be already broken in those file system that add LF into the lines?
So how about these changes:
if (CVS_STAT (logfile, &statbuf) < 0)
error (1, errno, "cannot find size of log file %s", logfile);
#ifdef BROKEN_READWRITE_CONVERSION
if ((logfd = CVS_OPEN (logfile, O_RDONLY | OPEN_BINARY)) < 0)
error (1, errno, "cannot open log file %s", logfile);
#else
if ((logfd = CVS_OPEN (logfile, O_RDONLY)) < 0)
error (1, errno, "cannot open log file %s", logfile);
#endif
Or would this be too much?
Best Regards,
Jari
