Hi Carl, On Thu, Feb 20, 2020 at 02:00:09PM +0000, Carl Henrik Holth Lunde wrote: > > The patch looks good at first glance, however I must have a deeper > > check because I have no idea what these unique_ids are used for :-) > > Maybe we could even completely drop them! > > > I do not know if they have any real-world use, but the use case is to > change ACLs after the configuration is loaded. I assume `-u` is > supported to allow an administrator to manually number an ACL (i.e. -u > 100), and then later change that ACL. OpenShift does not do this. > > The patch was written under the assumption that you wanted exactly the > same behavior.
Yes that's preferable indeed. > Since you say we maybe can drop them, I have created a > new patch which should work fine but does not guarantee that automatic > ids form a gapless sequence from 0. This simplifies the algorithm > csignificantly. The downside is that an overflow can happen if a very > high -u number is used in the config file, so I added a check for that. > The maximum is still very high. The problem now becomes a matter of backwards compatibility as it will refuse to start on some configs otherwise valid on older versions. I'd rather not do this. We could have simply detected the overflow in the automatic assignment but I'd rather keep the feature even if I don't use it myself. I've looked at the doc and figured what it's used for. It's for when you want to replace ACL or map values from the CLI. There is not always a unique reference file name to work with, but there's a unique ID that can be used prefixed by a '#'. It's possible that some infrastructures automatically update usage thresholds from the CLI using this, for example. > > I don't know how the current code deals with duplicated IDs nor what > > impact these might have. > > > > Duplicate IDs are handled when the configuration file is read. That > code is not efficient either but I did not look at improving that > because I do not know if anyone uses this feature *and* use very large > configuration files. For small hand written files the code is fine. Agreed. I've seen the pat_ref_lookupid() which does linear search. I'm not that much worried by this considering that you managed to significantly shrink the startup time in your case already. Well, after having looked at it more closely, I'd still rather keep the hole-filling algorithm, so if you have a v3 between the two versions it would be great :-) Thanks! Willy
