[Eug-lug] LP64

Jacob Meuser jakemsr at jakemsr.com
Mon Jan 10 15:55:20 PST 2005


On Mon, Jan 10, 2005 at 12:30:40AM -0800, Neil Parker wrote:
> Jacob Meuser wrote,
> >the thing is, there's also a foo_open function, that returns a
> >pointer to a struct.  the code was apparently written to make
> >open and ibp_open interchangable.  since they have different
> >return types, the author chose to use (void *).  the only use
> >of data is "if(!data) { ... }".
> 
> In that case, the correct approach is probably to wrap open() with
> something that returns a pointer.  For example,
> 
> #include <stdio.h>
> #include <sys/types.h>
> #include <sys/stat.h>
> #include <fcntl.h>
> #include <stdarg.h>
> #include <stdlib.h>
> 
> int *wrapped_open(cont char *name, int flags, ...)
> {
> 	int *fdp = (int *)malloc(sizeof(int));
> 	mode_t mode;
> 	va_list v;
> 
> 	if(fdp!=(int *)NULL) {
> 		if(flags&O_CREAT) {
> 			va_start(v, flags);
> 			mode = va_arg(v, mode_t);
> 			va_end(v);
> 			*fdp = open(name, flags, mode);
> 		} else
> 			*fdp = open(name, flags);
> 	}
> 	return fdp;
> }
> 
> Then (unless I made a coding error), you can call wrapped_open() instead of
> open(), and it will return a pointer the file descriptor instead of the
> descriptor itself (unless you're out of memory, in which case it returns
> NULL).  And gcc should be happy to cast the result of wrapped_open() to
> void *.
> 
> Of course you'll have to free() the returned pointer after you close the
> file.  You'll probably also have to rewrite some of the code that uses the
> return value, since the file descriptor is now "*data" instead of
> "(int)data".

thanks, this is helpful.

> I still don't understand why the original code did the cast, though.  The
> only reason to make the returns from open() and ibp_open() interchangeable
> is if you're planning to interchange them.  But it doesn't make sense to
> pass your fake pointer from open() to a routine that expects ibp_open()'s
> structure pointer, since the fake pointer doesn't actually point to the
> right structure.
> 
> I suppose the routines that handle ibp_open()'s structure could be written
> to call different code when they detect that the pointer has a nonsensical
> value, but that strikes me as more of an ugly hack than a reasonable
> design.

there's definitely some, uh, interesting, stuff in the IBP/LoRS
codebase.

> And are you SURE that "data" is only used in lines like "if(!data) ..."?  In
> that case, why is the code bothering to open the file at all?  The usual
> reason for opening a file is to do some kind of I/O on it...

now that I look closer, there are some reads and write associated with
data.

here's how _that_ gets setup

  ssize_t (*xio_read_v)(void *stream)(void *buf, size_t count);

if using read()

  xio_read_v = (ssize_t(*)(void *, void *, size_t))&read;

if using ibp_read()

  xio_read_v = &ibp_read;

-- 
<jakemsr at jakemsr.com>


More information about the EUGLUG mailing list