From owner-freebsd-hackers@freebsd.org Wed Aug 26 17:13:39 2020 Return-Path: Delivered-To: freebsd-hackers@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id BD2AD3B3844 for ; Wed, 26 Aug 2020 17:13:39 +0000 (UTC) (envelope-from jdavidlists@gmail.com) Received: from mail-lf1-x144.google.com (mail-lf1-x144.google.com [IPv6:2a00:1450:4864:20::144]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "GTS CA 1O1" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4BcCBC0gpCz4KtJ; Wed, 26 Aug 2020 17:13:38 +0000 (UTC) (envelope-from jdavidlists@gmail.com) Received: by mail-lf1-x144.google.com with SMTP id v12so1382480lfo.13; Wed, 26 Aug 2020 10:13:38 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=BtSeMX8StwXVJ+PYr+cR9tzW49MwLk+mThHomuZ6lRA=; b=VyDCXkqHr7B7Q900WRNY5n9F05CzVOscPjMW1rVBr3/HDBKrvv5QrPCVGk2JNDkZSB RTWC92RM7nIEsFAbKxaRrGL0IeL6oU1oeT82F+cIT7oxTRfQ7mQdEOrpm9Nq0EuQjHMn K6F8nNZ2Z3NBUAtOPu2w11A2bQutaE8iuqrgV0JHvSMzeck2T8NzPMAOAh6wV2ajGG4S IffvhndMI31XvTn2fGOCYRf7N8ynWhZXExdsLUPaEwyyHxf0+8DVQEeefzfWQIBHo5Jd G8Z3Vw+H5wwuFWwk30tjC103Y5vjivpAN58frUSu/E3xnovVB7izmLHUzu/ztUyxau5r +NbQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=BtSeMX8StwXVJ+PYr+cR9tzW49MwLk+mThHomuZ6lRA=; b=ibyZnhabkJb5nT+EjRrGGinaSxu247cJ0TY+sDucxYMu7a0VDvTnbaty70xjkzoIUb Fkyv49Fw0ZKSiMIUY9fr+e3N0lJxqCkESzkeexAt3UwlNw6Np47vOPhveB70VYjCk721 WWzvaO8PWJixCJjw1hOAk3Noubj7ft7OicTjD8eN91kln9Eh4lNjqFQIAipbLOXrkcgf Pv9hSoYNw+Br4EcWgOhrluTen6+vM3sYORKgUP/ibZ6lEVqcJZU78mUU/Li5XhPaedC8 N7rE9Om0Akm0TfkB7pR0uNTbCZ3rYIKGSpmWH8DDOIQsETT36/cO87vQXl5F3bpX0UVk mlBg== X-Gm-Message-State: AOAM5313F0oMn5DpQTIFo5hh2Vy6X8mum5MHwFjy5DMTUsBiRuELTrkV RRiNgVIa0oxjkGeiDTnOO4P7zLtc9Vfyo+JW6wd5wN7Ut1c= X-Google-Smtp-Source: ABdhPJzf/JmKI67Pa2xoZ/1GdtfHLNXGfit2bN3KXrHWhjUcAESqlDWbuPHVhFWYbRuq0LZ+kUk9n6S+lElIiPlubLQ= X-Received: by 2002:a19:644:: with SMTP id 65mr7713911lfg.143.1598462015539; Wed, 26 Aug 2020 10:13:35 -0700 (PDT) MIME-Version: 1.0 References: <20200826144013.GV2551@kib.kiev.ua> In-Reply-To: <20200826144013.GV2551@kib.kiev.ua> From: J David Date: Wed, 26 Aug 2020 13:13:24 -0400 Message-ID: Subject: Re: pidfile_open() usage in "mount" To: Konstantin Belousov Cc: freebsd-hackers@freebsd.org Content-Type: text/plain; charset="UTF-8" X-Rspamd-Queue-Id: 4BcCBC0gpCz4KtJ X-Spamd-Bar: --- Authentication-Results: mx1.freebsd.org; dkim=pass header.d=gmail.com header.s=20161025 header.b=VyDCXkqH; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (mx1.freebsd.org: domain of jdavidlists@gmail.com designates 2a00:1450:4864:20::144 as permitted sender) smtp.mailfrom=jdavidlists@gmail.com X-Spamd-Result: default: False [-3.66 / 15.00]; RCVD_TLS_ALL(0.00)[]; ARC_NA(0.00)[]; R_DKIM_ALLOW(-0.20)[gmail.com:s=20161025]; NEURAL_HAM_MEDIUM(-0.97)[-0.967]; FROM_HAS_DN(0.00)[]; TO_DN_SOME(0.00)[]; FREEMAIL_FROM(0.00)[gmail.com]; TO_MATCH_ENVRCPT_ALL(0.00)[]; MIME_GOOD(-0.10)[text/plain]; DWL_DNSWL_NONE(0.00)[gmail.com:dkim]; NEURAL_HAM_LONG(-1.00)[-1.003]; R_SPF_ALLOW(-0.20)[+ip6:2a00:1450:4000::/36]; DKIM_TRACE(0.00)[gmail.com:+]; RCPT_COUNT_TWO(0.00)[2]; RCVD_IN_DNSWL_NONE(0.00)[2a00:1450:4864:20::144:from]; NEURAL_HAM_SHORT(-0.69)[-0.686]; DMARC_POLICY_ALLOW(-0.50)[gmail.com,none]; FROM_EQ_ENVFROM(0.00)[]; MIME_TRACE(0.00)[0:+]; FREEMAIL_ENVFROM(0.00)[gmail.com]; ASN(0.00)[asn:15169, ipnet:2a00:1450::/32, country:US]; TAGGED_FROM(0.00)[]; MAILMAN_DEST(0.00)[freebsd-hackers]; RCVD_COUNT_TWO(0.00)[2] X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.33 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 26 Aug 2020 17:13:39 -0000 On Wed, Aug 26, 2020 at 10:40 AM Konstantin Belousov wrote: > It is possible that both pidfile_open() and the new function would share > some significant amount of code. Not as much as one might hope, it looks like. There's already a pidfile_read() function that isn't exported, so that helps. And it looks like it might be worthwhile to pull out some of the path-handling code to a common static function. But if for no other reason than the O_WRONLY|O_TRUNC flags, the code necessarily diverges pretty hard after the flopen/flopenat (depending on version). Would the right approach to this look something like: - New static function: int pidfile_real_open( const char *pathp, mode_t ) that opens the right file, using the code & logic from lines existing pidfile_open() (pidfile.c lines 105 - 147 in HEAD). - Modify pidfile_open() to use pidfile_real_open() - New public function: int pidfile_get( const char *pathp, pid_t *pidptr, int ndelay ) returns 0 on success, EWOULDBLOCK (if ndelay is nonzero and lock fails) or ENOENT (if pidfile does not exist). Basically a wrapper around pidfile_open_int() and pidfile_read(). Notes: - pidfile_real_open: don't love the name; not sure if there is a naming convention for static functions that are called by public ones. - pidfile_real_open: different versions will be need for 12/HEAD and pre-12, due to lack of flopenat() in earlier releases. - pidfile_get: returns error value directly as I think the general move is away from returning -1 and setting errno in libraries like this - pidfile_get: ndelay would basically control the decision whether to skip (if nonzero) the nanosleep loop structure around pidfile_read, as happens in pidfile_open(). Probably not needed in most cases, but I've been bitten by unskippable sleeps before in the weirdest places, so why not? - pidfile_get: Would rather call this pidfile_read but that's taken. Since it's static though, could rename pidfile_read to pidfile_real_read (or whatever else is conventional, same as pidfile_real_open) and call this pidfile_read. That's my preference, but it's hard for me to tell what's actually better and what just looks better to me. If this all sounds OK, it shouldn't take me too long to come up with a patch and corresponding test(s). Thanks!