Get rid of transitional lfs API #819

Merged
merged 1 commit into from Oct 7, 2012

Projects

None yet

4 participants

@jpf91
Contributor
jpf91 commented Sep 28, 2012

Those kind of functions should be placed in druntime, but the *64 (transitional) functions as a public interface are actually deprecated and shouldn't be used anymore.

Druntime takes care that the normal stat, fopen,... calls support large files, if __USE_FILE_OFFSET64 is defined (which is what the new C API does as well)

Note: It's actually important to use the functions from core.sys.posix.stdio as the functions in core.stdc.stdio are not large file aware. This is probably a bug in druntime.

Note 2: I hope that the new stdio/File will work better if no lfs support is available. The current implementation probably won't even compile if off_t is 32bit.

BTW: Is there a place where I could write down some documentation on implementation details?
Large file support is a mess and it would probably be a good idea to document our implementation somewhere.

@ibuclaw
Member
ibuclaw commented Sep 29, 2012

LGTM

@ibuclaw
Member
ibuclaw commented Sep 29, 2012

Does this pull depend on dlang/druntime#310 ?

@jpf91
Contributor
jpf91 commented Sep 29, 2012

@ibuclaw No, the druntime pull request only syncs the declarations with the C headers, but there's no real effect as we currrently define both __USE_FILE_OFFSET64 and __USE_LARGEFILE64. Phobos does not use statvfs, so those changes don't affect us either.

@andralex
Member

This would break code that uses the old names, is that correct? Any documented names that have been removed?

For documenting lfs support, we can set up a dedicated webpage if necessary. For now you may want to embed comments in the code as needed.

@jpf91
Contributor
jpf91 commented Sep 29, 2012

For D user code the only public API that changed is that struct_stat64 is replaced by the druntime definition stat_t. We could provide an alias, both types should actually be identical.

If you're worried about old systems which support the transitional API, but not the new one: I don't think we support such systems. The new API is a standard since 1996, Apple already calls the transitional API deprecated, linux/glib supports the new API since linux 2.4, FreeBSD doesn't even support the transitional API.

@andralex
Member

We need to provide a struct_stat64 alias then.

@jpf91
Contributor
jpf91 commented Sep 29, 2012

OK, added the alias. I also added a

 pragma(msg, "struct_stat64 is deprecated, please use stat_t instead");

but as file is used a lot in phobos, that message is kinda annoying.

@andralex
Member

The pragma won't be popular. What's wrong with deprecated?

@jpf91 jpf91 Get rid of transitional lfs API
Those kind of functions should be placed in druntime,
but the *64 (v) functions as a public interface are actually
deprecated and shouldn't be used anymore.

Druntime takes care that the normal stat, fopen, calls support
large files, if __USE_FILE_OFFSET64 is defined (which is what
the new C API does as well)

Note: It's actually important to use the functions from
core.sys.posix.stdio as the functions in core.stdc.stdio
are not large file aware. This is probably a bug in druntime.
f989b21
@jpf91
Contributor
jpf91 commented Sep 30, 2012

OK, I just marked it as deprecated then. Some people have been recommending a 2 phase deprecation in phobos, first pragma(msg), then deprecated and then removal.

@alexrp
Member
alexrp commented Oct 6, 2012

The likelihood of these actually being used in normal D code is so low that I think deprecated is fine.

LGTM.

@andralex andralex merged commit a37e919 into dlang:master Oct 7, 2012

1 check was pending

default Pass: 4, Pending: 5
Details
@andralex
Member
andralex commented Oct 7, 2012

merged, thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment