linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff Layton <jlayton@redhat.com>
To: Mike Frysinger <vapier@gentoo.org>
Cc: libc-alpha@sourceware.org, linux-fsdevel@vger.kernel.org,
	Michael Kerrisk <mtk.manpages@gmail.com>,
	Carlos O'Donell <carlos@redhat.com>,
	Yuriy Kolerov <Yuriy.Kolerov@synopsys.com>
Subject: Re: [glibc PATCH] fcntl: put F_OFD_* constants under #ifdef __USE_FILE_OFFSET64
Date: Wed, 17 Aug 2016 15:15:04 -0400	[thread overview]
Message-ID: <1471461304.3196.101.camel@redhat.com> (raw)
In-Reply-To: <20160817184333.GC21655@vapier.lan>

On Wed, 2016-08-17 at 11:43 -0700, Mike Frysinger wrote:
> On 17 Aug 2016 10:47, Jeff Layton wrote:
> > 
> > The Linux kernel expects a flock64 structure whenever you use OFD locks
> > with fcntl64. Unfortunately, you can currently build a 32-bit program
> > that passes in a struct flock when it calls fcntl64.
> > 
> > Only define the F_OFD_* constants when __USE_FILE_OFFSET64 is also
> > defined, so that the build fails in this situation rather than
> > producing a broken binary.
> 
> this seems to be going against the glibc API/guarantees we've provided
> before (or at least tried to promise), and what the fcntl(2) man page
> says now.  namely, we haven't documented F_GETLK64 or struct flock64,
> with the expectation that the user just calls fcntl() with a struct
> flock.  in fact, the man page even goes so far as to discourage people
> from using the *64 variants.
> 
> it should be possible using our existing LFS framework to make the OFD
> cmds available even to 32-bit apps (where sizeof(off_t) == 32).  but
> maybe the usage of F_GETLK64/struct flock64/etc... in the real world
> has made it hard to put that genie back in the bottle ?  we'd have to
> version the current fcntl symbol, create a new fcntl symbol that does
> 32->64 munging, and add a new fcntl64 symbol that we'd transparently
> rewrite to when LFS is turned on.
> -mike


There should be no need to use struct flock64 explicitly, and there is
already a proposed patch to fix the manpage accordingly.

What we _do_ want to ensure is that large file offsets are in use if
the application wants to use OFD locks (either by virtue of being on a
64 bit arch, or by defining _FILE_OFFSET_BITS=64).

In principle, we could try to fix it up so that the kernel can handle
OFD locks with legacy struct flock. That would mean adding
F_OFD_SETLK64 and friends in both the kernel and glibc, and we'd have
to ensure that legacy kernel+new glibc is handled sanely (and vice-
versa). That's a lot of effort (and more risk for breakage) to handle a
use case that I'm not sure even exists. This approach is much simpler,
and we'll just be breaking at build time a case that was already broken
at runtime.

In hindsight, I wish I had just introduced F_OFD_SETLK64 and friends to
make them work with legacy struct flock when I did these patches (mea
culpa!), but I don't really see the value in doing that at this point.

-- 
Jeff Layton <jlayton@redhat.com>

  reply	other threads:[~2016-08-17 19:20 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-08-17 14:47 [glibc PATCH] fcntl: put F_OFD_* constants under #ifdef __USE_FILE_OFFSET64 Jeff Layton
2016-08-17 15:44 ` Joseph Myers
2016-08-17 17:49   ` Jeff Layton
2016-08-17 17:56     ` Joseph Myers
2016-08-17 18:23       ` Jeff Layton
2016-08-17 16:13 ` Mike Frysinger
2016-08-17 17:34 ` Florian Weimer
2016-08-17 17:39   ` Jeff Layton
2016-08-17 18:02     ` Florian Weimer
2016-08-17 18:21       ` Jeff Layton
2016-08-17 18:51         ` Florian Weimer
2016-08-17 19:20           ` Jeff Layton
2016-08-18  8:44             ` Florian Weimer
2016-08-18  8:58               ` Andreas Schwab
2016-08-17 20:52           ` Andreas Schwab
2016-08-18  8:45             ` Florian Weimer
2016-08-17 18:43 ` Mike Frysinger
2016-08-17 19:15   ` Jeff Layton [this message]
2016-08-17 19:59     ` Michael Kerrisk (man-pages)
2016-08-17 20:05       ` Jeff Layton
2016-08-17 20:37         ` Mike Frysinger
2016-08-17 20:57           ` Jeff Layton
2016-08-17 21:35             ` Mike Frysinger
2016-08-17 21:48               ` Jeff Layton
2016-08-18  9:00                 ` Florian Weimer
2016-08-23 11:03                   ` Cyril Hrubis
2016-08-23 11:36                     ` Jeff Layton
2016-08-23 11:38                       ` Cyril Hrubis
2016-08-23 21:10                         ` Michael Kerrisk (man-pages)
2016-11-14 13:45                           ` Cyril Hrubis
2016-11-22 18:41                             ` Florian Weimer
2016-08-18  8:57             ` Florian Weimer
2016-08-17 20:03     ` Mike Frysinger
2016-08-17 21:30       ` Cyril Hrubis

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1471461304.3196.101.camel@redhat.com \
    --to=jlayton@redhat.com \
    --cc=Yuriy.Kolerov@synopsys.com \
    --cc=carlos@redhat.com \
    --cc=libc-alpha@sourceware.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=mtk.manpages@gmail.com \
    --cc=vapier@gentoo.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).