[Libburn] Patch to enable arbitrary input fd (updated 2nd submission)

scdbackup at gmx.net scdbackup at gmx.net
Sat Feb 11 01:16:51 PST 2006


Hi,

> sizeof(off_t) on a 32 bit machine is 32 bits...

Not necessarily. It is 32 bits if nobody bothers to
enable large file support.

Quoting myself from a discussion on the cdwrite list:
"
-D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE
...
It is said that the macros are described in  info libc
but i am not masochist enough to find out why my  info
only shows me its top level table of contents then.
Found by google:
  http://www.gnu.org/software/libc/manual/html_node/Feature-Test-Macros.html
I once got my inspirations from :
  http://www.suse.de/~aj/linux_lfs.html
which advises to define all three macros.
"
[the third macro would be _LARGEFILE64_SOURCE]


If you do not enable large file support then you will
not get happy with DVD sized files anyway.

off_t is the data type witch matches the mental
capacity of the file system access functions.
It is used by nearly any of those functions which
have to deal with file sizes. (There are some like
fseek() which is stuck to long, nevertheless. Here
one should use the newer fseeko().)

I would accept any other sufficient type here.


> I'm thinking maybe we should consider C99 instead of C90, which gives us 
> int64_t in stdint.h. (I've added that #include)

Type double would be a solution even for K&R C. :))

Usually i would heavily object a code transition 
which does not allow to port to many other Unix-like
systems. But libburn is tied so close to modern Linux
that there will be not much of a practical problem
with that.

My own application code will not go further than ANSI-C.

 
> What I merged was your new blocking file_read.
> 
> Can you change the rest to use off_t?

I am not sure wether i understand what you want of me.
Is this a rethorical question to underline the problems
introduced by a use of off_t ?

Currently  file_size()  adds an off_t to an int. That
off_t is obtained from fstat(). If off_t is 64 bit
(which can be caused by system settings) then you get
into trouble with very large files.
If off_t is 32 bit than that type itself will have
trouble with big files and it will confuse int if
the file size is between 2 GB and 4 GB.

Big files may exist even in ext2 systems (up to 4 GB)
of a 2.2 Linux from 1999.

My application will currently prevent fixed sizes of
more than 2 GB . But a protection against overly large
file sizes would make it necessary to check the file
with large file support enabled and to reject it if it
is too large.
So we can just enable large file support from start
and make libburn safe against int-off_t interaction.

Yes, i would help to identify the spots in the code
which need mending. 


> Also, I don't want a change to structure.c for this...  Instead of the 
> burn_source_get_size() wrapper, can you just write your own get_size 
> function instead of using file_size in burn_fd_source_new()?

But that would make it necessary to change the
construtor of  burn_source  in order to attach a
different function.

How about this:
One could transfer the functionality of 
burn_source_get_size() into file_size().
This function needs mending anyway.


> The fixed_size data shouldn't be a new member of the burn_source struct, 
> rather you use the source specific data pointer (*data) that's already in 
> burn_source.  (and also add the appropriate function to free it)

Huh ?
Wouldn't i start a new inofficial data structure by that
and how would i make the functions of libburn recognize
that new structure properly ? What happens if the next
programmer comes and wants to put other information into
data ?

If i add a simple data type to burn_source then it is
quite clear what is meant.

But if i add it to *data i would probably as a consequence
have to do things like
  my_private_struct.fixed_size = size;
  source->data = (void *) &my_private_struct;
and
  size = ((struct my_private_struct_type *) source->data)->size;

Compare this with
  source->size = size;
and
  size = source->size;

The fixed size is intended as a new feature of burn_soure
which may apply to any source and not as some peculiarity
of some data sources.
I would say that it deserves a decent variable and should 
not be pushed into a dark corner like (void *).


> The only change we should need in libburn.h would be your
> +struct burn_source *burn_fd_source_new(int fd, int64_t size);
> 
> Does that make sense?

Not completely yet. It is too early in the morning.


Summary of my thoughts:

- We can use quite any type rather than off_t. We could stay with int.

- If you don't like  burn_source_get_size()  then let me transfer its
  functionality into  file_size() .

- Please let me introduce  burn_source.fixed_size  (with what type ever).


Have a nice day :)

Thomas




More information about the libburn mailing list