Testing file offset bits?

[Alexander Neundorf]

On Monday 22 April 2013, Stephen Kelly wrote:
> On 04/21/2013 03:05 PM, Alexander Neundorf wrote:
> > On Sunday 21 April 2013, Alexander Neundorf wrote:
> >> On Saturday 20 April 2013, Stephen Kelly wrote:
> >>> On 02/27/2013 03:59 PM, David Faure wrote:
> >>>> On Monday 18 February 2013 12:10:31 Dirk Müller wrote:
> >>>>> 2013/2/16 Alexander Neundorf <neundorf at kde.org>:
> >>>>>> Dirk Mueller added it in 2008:
> >>>>>> http://websvn.kde.org/?view=revision&revision=829068
> >>>>>> 
> >>>>>> If I remove every compiler flag where I'm not sure why it is needed,
> >>>>>> we'll be left with not much.
> >>>>> 
> >>>>> This flag is needed in order to be able to support files > 2GB even
> >>>>> on 32bit platforms. The default is to use the non-64bit aware
> >>>>> syscalls on 32bit platforms, which makes every application to
> >>>>> SIGXFSZ (which is equivalent to a crash) when they see or touch a
> >>>>> file > 2GB.
> >>>>> 
> >>>>> This is an excellent default for upstream, but I decided back then
> >>>>> that KDE apps should be big-file aware right from the start. The
> >>>>> reason why this was implemented as a configure check rather than an
> >>>>> universation global setting is that back then there was btw software
> >>>>> that did not
> >>>>> compile properly on x86_64 when _FILE_OFFSET=64 was set (which was a
> >>>>> bug).
> >>>> 
> >>>> This is only necessary when using ::open() directly though (or the
> >>>> other stuff from kde_file.h). I'm porting the code away from
> >>>> kde_file.h (to QFile / QFileInfo) whereever necessary, which makes
> >>>> this unneeded.
> >>>> 
> >>>> Maybe only in kio_file, in the end, for performance reasons.
> >>> 
> >>> I've moved it to kdecore for now.
> >>> 
> >>> Should the -D_LARGEFILE64_SOURCE definition also be moved?
> >> 
> >> IMO having these flags present by default does not hurt (except in those
> >> cases which were tested for), and when needed, avoid bugs when dealing
> >> with big files, so I would leave them there.
> 
> In my opinion, having definitions set globally where not needed is a
> 'bad code smell'.
> 
> > Doesn't removing such a definition potentially break any user of kdelibs
> > which (maybe unknowingly) rely on it being present ?
> > (With KDE frameworks, using KDECompilerSettings.cmake is optional anyway,
> > mostly for convenience).
> 
> Users of kdelibs will be using either Qt file APIs or something from
> kde_file.h, which will still use the definition.
> 
> If we use our imagination, we could say that potentially someone could
> be using open directly and relying on the definition having been set
> elsewhere. That would not be smart on their part, is vanishingly rare
> (because obviously they'd use Qt APIs instead), and I don't think we
> should care.

We are not talking about building kdelibs or even KDE SC here, we are talking 
about everything out there building against kdelibs.
They all get those 64-bit-related definitions with KDE4, and I think they also 
got them with KDE3 and maybe also KDE2.

Nobody is forced to use KDECompilerSettings.cmake anymore with KDE frameworks.
This file contains optional compiler settings, which are considered useful. If 
a developer wants to take care of this himself, he simply does not have to use 
that file.

Why break apps which may rely on those flags being present and force them to 
take care of this themselves, probably only after some user reported breakage 
on large files after upgrading ?

You may say that direct usage of open() etc. is considered bad in kdelibs or 
KDE SC, but it is valid POSIX API, and any 3rd party developer is free to use 
it and I'm far from saying that it is per se a bad thing.

Alex