[m-dev.] for review: change MercuryFile structure

[Peter Ross]

On Fri, Aug 11, 2000 at 10:08:08PM +1000, Fergus Henderson wrote:
> On 11-Aug-2000, Peter Ross <peter.ross at miscrit.be> wrote:
> > Add a variant of the MercuryFile structure that it contains pointers to
> > functions which operate on the MercuryFile.  This allows us, for
> > example, to create a MercuryFile structure which operates on sockets and
> > use the predicates in io.m to operate on the socket stream.
> > 
> > This variant is turned on by passing --enable-new-mercuryfile-struct to
> > configure.
> 
> OK, the basic structure of this diff looks good.
> So this time I'm going through the code in a bit more detail.
> 
> ...
> > +++ logged_output.m	Fri Aug 11 21:01:59 2000
> > +:- pragma c_code(create_stream(FileName::in, IOStream::out, IO0::di, IO::uo), "
> > +	MercuryFile	*stream;
> > +	FILE		*file;
> > +
> > +	file = fopen(FileName, ""w"");
> > +
> > +	incr_hp(stream, sizeof(MercuryFile));
> 
> The argument to incr_hp should be a size in Words, not a size in bytes.
> 

Changed to 
	incr_hp(stream, ((sizeof(MercuryFile) / sizeof(MR_Word)) + 1));

> > +	IOStream = (Word) stream;
> 
> s/Word/MR_Word/
> 
Done.

> > +int ME_logged_output_vfprintf(MR_StreamInfo *info,
> > +		const char *pszFormat, va_list ap);
> 
> s/pszFormat/format/
> 
Done.

> Even Microsoft have now realized that Hungarian notation was a mistake ;-) 
> The new .NET stuff does not use Hungarian notation.
> 
> > +int
> > +ME_logged_output_write(MR_StreamInfo *info, const void *buffer, size_t size)
> > +{
> > +	int rc;							       
> > +	fwrite(buffer, sizeof(unsigned char), size, stdout);
> > +	rc = fwrite(buffer, sizeof(unsigned char), size, info->file);
> > +}
> 
> You need `return rc;' at the end of this function.
> 

This is the line that should be there

	return (rc < size ? -1 : rc);

It is also not there in my second diff, even though it is in the file
very strange!

%------------------------------------------------------------------------------%
%------------------------------------------------------------------------------%

> io.m:
> > Index: library/io.m
> > ===================================================================
> > RCS file: /home/mercury1/repository/mercury/library/io.m,v
> > retrieving revision 1.200
> > diff -u -r1.200 io.m
> > --- library/io.m	2000/08/01 09:04:19	1.200
> > +++ library/io.m	2000/08/11 11:02:18
> > @@ -1494,7 +1494,7 @@
> >  		[will_not_call_mercury, thread_safe],
> >  "{
> >  	MercuryFile *f = (MercuryFile *) Stream;
> > -	clearerr(f->file);
> > +	clearerr(MR_file(*f));
> >  }").
> 
> Hmm, there are lots of places like this that access
> `MR_file' without checking what kind of file it is.
> If the file here is a user-defined file, then it
> might not have an associated `FILE *', in which case
> this code will have undefined behaviour, which is not good.
> I think you need to go through all the code looking for
> occurrences of MR_file(), and make them all do something
> reasonable in the case where MR_IS_FILE_STREAM() returns false.
> 
> > @@ -1650,7 +1662,12 @@
> >  	char *buffer = (Char *) Buffer0;
> >  	int items_read;
> >  
> > -	items_read = fread(buffer + Pos0, sizeof(Char), Size - Pos0, f->file);
> > +	if (MR_IS_FILE_STREAM(*f)) {
> > +		items_read = fread(buffer + Pos0, sizeof(Char), Size - Pos0,
> > +				MR_file(*f));
> > +	} else {
> > +		mercury_io_error(f, ""Attempted fread from non-file stream"");
> 
> Why do you call mercury_io_error() here?
> Rather than this if-then-else, shouldn't you just call the `MR_READ()' macro?
> 

Here is a relative diff of these changes (produced using interdiff):

--- library/io.m
+++ library/io.m
@@ -1497 +1497,6 @@
-       clearerr(MR_file(*f));
+
+       if (MR_IS_FILE_STREAM(*f)) {
+               clearerr(MR_file(*f));
+       } else {
+               /* Not a file stream so do nothing */
+       }
@@ -1665,6 +1670 @@
-       if (MR_IS_FILE_STREAM(*f)) {
-               items_read = fread(buffer + Pos0, sizeof(Char), Size - Pos0,
-                               MR_file(*f));
-       } else {
-               mercury_io_error(f, ""Attempted fread from non-file stream"");
-       }
+       MR_READ(*f, buffer + Pos0, Size - Pos0);
@@ -3016 +3016,7 @@
-       fseek(MR_file(*stream), Off, seek_flags[Flag]);
+       if (MR_IS_FILE_STREAM(*stream)) {
+               fseek(MR_file(*stream), Off, seek_flags[Flag]);
+       } else {
+               mercury_io_error(stream,
+                               "io__seek_binary_2: unseekable stream");
+       }
+
@@ -3026 +3032,6 @@
-       Offset = ftell(MR_file(*stream));
+       if (MR_IS_FILE_STREAM(*stream)) {
+               Offset = ftell(MR_file(*stream));
+       } else {
+               mercury_io_error(stream,
+                        ""io__binary_stream_offset: untellable stream"");
+       }

--------------------------------------------------------------------------
mercury-developers mailing list
Post messages to:       mercury-developers at cs.mu.oz.au
Administrative Queries: owner-mercury-developers at cs.mu.oz.au
Subscriptions:          mercury-developers-request at cs.mu.oz.au
--------------------------------------------------------------------------