[OpenAFS] Current "balance" practice?

[Benjamin Kaduk]

On Tue, Nov 27, 2018 at 04:51:54PM -0500, Jeffrey Altman wrote:
> On 11/27/2018 2:21 PM, Chaskiel Grundman wrote:
> > There is another problem beyond 64-bit safety. It appears that the some of
> > the openafs devs didn't learn from the project's own experience with the
> > linux developers, as
> >
> > extern afs_int32 vsu_ClientInit(int noAuthFlag, const char *confDir,
> >                                 char *cellName, afs_int32 sauth,
> >                                 struct ubik_client **uclientp,
> >                                 int (*secproc)(struct rx_securityClass *,
> > afs_int32));
> > 
> > 
> > in <= 1.6 has become
> > 
> > extern afs_int32 vsu_ClientInit(const char *confDir, char *cellName,
> >                                 int secFlags,
> >                                 int (*secproc)(struct rx_securityClass *,
> >                                                afs_int32),
> >                                 struct ubik_client **uclientp);
> > 
> > 
> > in 1.8. and I can't even use #ifdef VS2SC_NEVER to detect the change --
> > it's an enum.
> 
> That would be AuriStor's fault.  The change in question was
> 
>   commit 3720f6b646857cca523659519f6fd4441e41dc7a
>   Author: Simon Wilkinson <sxw@your-file-system.com>
>   Date:   Sun Oct 23 16:21:52 2011 +0100
> 
>       Rework the ugen_* interface
> 
> The vsu_ClientInit() signature change was a side-effect of the
> refactoring of ugen_ClientInit().  No one remembered the possible out of
> tree usage of vsu_ClientInit().  vsu_ClientInit() is not an exported
> function.  As such its status as public is murky at best.

Indeed, I use the export symbol lists for the public shared libraries to
determine what standard of review to apply to API changes, and non-exported
symbols mostly get a free-for-all for API changes.

> I suggest using the existence of one of these CPP macros as a test.
> They were added shortly after the vsu_ClientInit() signature change was
> merged.
> 
>   /* Values for the UV_ReleaseVolume flags parameters */
>   #define REL_COMPLETE    0x000001  /* force a complete release */
>   #define REL_FULLDUMPS   0x000002  /* force full dumps */
>   #define REL_STAYUP      0x000004  /* dump to clones to avoid offline
> time */
> 
> The introduction of enum vol_s2s_crypt came much later.
> 
> If you would prefer AuriStor can submit a change to restore the prior
> signature.

That said, I do not subscribe to the philosophy of "aggressively break APIs
not documented as stable", and would likely merge such a change if there
was demand for it.  (But I leave it to Chaskiel et al to express such a
demand, if one is indeed present.)

-Ben