[PATCH] Cache lookupname/sid in gencache

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] Cache lookupname/sid in gencache

Samba - samba-technical mailing list
Hi!

winbindd_cache.tdb is tightly coupled with a specific domain. For
lookupname and lookupsid this is not right, we can ask "our" DC for
names in any trusted domain. This patchset moves the cache for
lookupsid and lookupname into gencache.

Review appreciated!

Thanks, Volker

--
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-370000-0, fax: +49-551-370000-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.sernet.de, mailto:[hidden email]

patch.txt (51K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Cache lookupname/sid in gencache

Samba - samba-technical mailing list
On Tue, Nov 21, 2017 at 09:07:03AM +0100, Volker Lendecke via samba-technical wrote:
> winbindd_cache.tdb is tightly coupled with a specific domain. For
> lookupname and lookupsid this is not right, we can ask "our" DC for
> names in any trusted domain. This patchset moves the cache for
> lookupsid and lookupname into gencache.

reviewing now...

-slow

--
Ralph Boehme, Samba Team       https://samba.org/
Samba Developer, SerNet GmbH   https://sernet.de/en/samba/

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Cache lookupname/sid in gencache

Samba - samba-technical mailing list
In reply to this post by Samba - samba-technical mailing list
On Tue, Nov 21, 2017 at 09:07:03AM +0100, Volker Lendecke via samba-technical wrote:
> winbindd_cache.tdb is tightly coupled with a specific domain. For
> lookupname and lookupsid this is not right, we can ask "our" DC for
> names in any trusted domain. This patchset moves the cache for
> lookupsid and lookupname into gencache.
>
> Review appreciated!

nice!, and generally reviewed-by-me, but one thing that somewhat irritates me
enough so I'd like to raise it is the struct initializer in a function call
argument, ie

        ok = gencache_set_data_blob(
                keybuf, (DATA_BLOB) { .data = (uint8_t *)val,
                                      .length = talloc_get_size(val) },
                timeout);

I personally find this somewhat overloaded and would prefer having
argument-to-a-function initialisation as a seperate statement.

Does it make any difference in the generated code? What do others think?

-slow

--
Ralph Boehme, Samba Team       https://samba.org/
Samba Developer, SerNet GmbH   https://sernet.de/en/samba/

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Cache lookupname/sid in gencache

Samba - samba-technical mailing list
On Mon, Nov 27, 2017 at 11:56:43PM +0100, Ralph Böhme wrote:

>
>         ok = gencache_set_data_blob(
>                 keybuf, (DATA_BLOB) { .data = (uint8_t *)val,
>                                       .length = talloc_get_size(val) },
>                 timeout);
>
> I personally find this somewhat overloaded and would prefer having
> argument-to-a-function initialisation as a seperate statement.
>
> Does it make any difference in the generated code? What do others think?

It's highly unlikely that it makes a difference in generated code. We
could also use data_blob_const() here or add a separate variable. I
don't really care, but this to me is pretty dense and to the point.

Volker

--
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-370000-0, fax: +49-551-370000-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.sernet.de, mailto:[hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Cache lookupname/sid in gencache

Samba - samba-technical mailing list
On Tue, Nov 28, 2017 at 12:52:49PM +0100, Volker Lendecke wrote:

> On Mon, Nov 27, 2017 at 11:56:43PM +0100, Ralph Böhme wrote:
> >
> >         ok = gencache_set_data_blob(
> >                 keybuf, (DATA_BLOB) { .data = (uint8_t *)val,
> >                                       .length = talloc_get_size(val) },
> >                 timeout);
> >
> > I personally find this somewhat overloaded and would prefer having
> > argument-to-a-function initialisation as a seperate statement.
> >
> > Does it make any difference in the generated code? What do others think?
>
> It's highly unlikely that it makes a difference in generated code. We
> could also use data_blob_const() here or add a separate variable. I
> don't really care, but this to me is pretty dense and to the point.

too dense for my simple mind. :) So if you don't mind I'll add a helper variable
and push with that change.

-slow

--
Ralph Boehme, Samba Team       https://samba.org/
Samba Developer, SerNet GmbH   https://sernet.de/en/samba/

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Cache lookupname/sid in gencache

Samba - samba-technical mailing list
On Tue, Nov 28, 2017 at 01:02:12PM +0100, Ralph Böhme wrote:
> > It's highly unlikely that it makes a difference in generated code. We
> > could also use data_blob_const() here or add a separate variable. I
> > don't really care, but this to me is pretty dense and to the point.
>
> too dense for my simple mind. :) So if you don't mind I'll add a helper variable
> and push with that change.

Sure, thanks!

Volker

--
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-370000-0, fax: +49-551-370000-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.sernet.de, mailto:[hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Cache lookupname/sid in gencache

Samba - samba-technical mailing list
On Tue, Nov 28, 2017 at 01:03:16PM +0100, Volker Lendecke wrote:
> On Tue, Nov 28, 2017 at 01:02:12PM +0100, Ralph Böhme wrote:
> > > It's highly unlikely that it makes a difference in generated code. We
> > > could also use data_blob_const() here or add a separate variable. I
> > > don't really care, but this to me is pretty dense and to the point.
> >
> > too dense for my simple mind. :) So if you don't mind I'll add a helper variable
> > and push with that change.
>
> Sure, thanks!

pushed.

-slow

--
Ralph Boehme, Samba Team       https://samba.org/
Samba Developer, SerNet GmbH   https://sernet.de/en/samba/