headers
Jeremy Allison | 1 May 2009 02:19
Picon
Favicon

Re: [PATCH] Change unix_convert to use struct smb_filename

On Thu, Apr 30, 2009 at 12:02:32PM -0700, Tim Prouty wrote:
>
> Thanks for the feedback!
>
> I was considering that option, but since we already had a talloc_ctx  
> being passed around, it seemed simpler to just have the smb_filename  
> struct sit on the stack.  I guess one possible advantage of tallocing an 
> smb_filename struct is that the talloc_ctx arg could be eliminated from 
> unix_convert, further simplifying the API.

Actually you can't do that as you sometimes might want to
talloc one *not* on talloc_tos() (which I'm assuming you
were intending using instead - using the NULL context is
not recommended). So you still need the talloc_ctx argument
to unix_convert() even in the talloc'ed struct smb_filename
returned case.

> Another advantage is that the memory would be more cleanly tracked, and 
> could be freed earlier as soon as it is no longer being used.  Are there 
> other advantages as well?
>
> On a related note, in a future patch it would make sense to store the  
> smb_filename struct in the file_struct as well.  To add this ability  
> I'll probably also add some utility functions that alloc/init/copy/etc  
> smb_filename structs.  These utility structs could be used here as well.

Ok, I went over this patch and I really like it (although
it won't apply with git am to master anymore :-).

I agree with Volker. unix_convert should return a talloc'ed
(Continue reading)

Permalink | Reply | 0 comments |
headers
Christian Perrier | 1 May 2009 07:57
Picon
Favicon
Gravatar

Re: STatus update on Bugzilla.samba.org

Quoting Gerald Carter (jerry <at> plainjoe.org):
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Hey all,
> 
> Big thanks to Deryck for quick fixes to bugzilla.samba.org.
> 
>  * The "Too many connections" was the result of a bot.
>    Fixed in robots.txt now.

Just in case: that bot is not a Debian one, right? We have some bots
that track upstream BTS to get proper status information about bugs
that are linked between Debian BTS and upstream BTS.

This very bot, named bts-link is run from one of the *.debian.org
machines, I d'ont exactly know which one.

> 
>   * Inability to view attachments was a missing template
>     variable after the upgrade.
> 
> Both have been resolved now.  There's a few outstanding issues
> that appear to just be more template fixes.  We'll work
> on resolving those as well this week.

Thanks for the fix. That will help a lot..:-)
Permalink | Reply | 1 comment |
headers
tridge | 1 May 2009 08:05
Picon
Favicon
Gravatar

Re: thread pool helpers

Hi Jerry,

 >    fd = open()
 >    secdesc = GetSecurityDescriptor(fd)
 >    if (!RtlAccessCheck(token, secdesc)) {
 >       close(fd)
 >    }
 >    SaveFdToFileHandle(fd)
 > 
 > Do you agree?

yep, for opens that don't imply O_CREAT this can work, unless the
system has a device driver installed that takes an undesirable action
on open() on a device inode. That would be pretty unusual.

You can also use O_NOFOLLOW to reduce the chance of this problem
happening on systems that support it.

 > The create/overwrite is a little tricker.  I'll have
 > to think about an answer for those cases more.

One method that would beat the symlink race condition is this:

 1) break the path into directory and name components, then always
 chdir() to the directory first. After the chdir check you ended up
 where you expected to.

 2) once in the directory, try the open with O_NOFOLLOW, if that works
 or if the open fails with something other than -1/ELOOP then you're
 done.
(Continue reading)

Permalink | Reply | 0 comments |
headers
Volker Lendecke | 1 May 2009 10:22
Picon
Favicon

Re: [PATCH] Change unix_convert to use struct smb_filename

On Thu, Apr 30, 2009 at 12:02:32PM -0700, Tim Prouty wrote:
> I was considering that option, but since we already had a talloc_ctx  
> being passed around, it seemed simpler to just have the smb_filename  
> struct sit on the stack.  I guess one possible advantage of tallocing  
> an smb_filename struct is that the talloc_ctx arg could be eliminated  
> from unix_convert, further simplifying the API.

No, I mean unix_convert should create smb_filename, changing
the argument to "struct smb_filename **smb_fname" :-)

> I'm not familiar with the talloc_pool trick.  Are there examples of  
> the trick in use elsewhere?

I'd use this only if this turns to be out to be a noticeable
performance hit. Just allocate a talloc_pool that can carry
all the children's memory, and allocating the children won't
call malloc(3) anymore.

Volker
Permalink | Reply | 0 comments |
headers
Volker Lendecke | 1 May 2009 12:38
Picon
Favicon

Re: thread pool helpers

On Tue, Apr 28, 2009 at 05:47:13PM +1000, tridge <at> samba.org wrote:
> I think you've created a really nice API, but the brokenness of
> pthreads make me think that we're better off implementing that API
> either on top of fork() or on top of a raw clone() call.

I've pushed it now as is.

There might be pthreads implementations around that are not
as broken. Maybe the FreeBSD one that Isilon is based upon
is better in that respect, but Tim and Steven should comment
here.

You can disable it and replace it with a sync replacement.

I've looked at a clone based implementation of
fncall_send/recv that coordinates via pipes. My main problem
here is to find a way to implement memory barriers to make
sure we see all helper threads' memory contents correctly.
Do the pipe syscalls guarantee such a memory barrier?

Volker
Permalink | Reply | 0 comments |
headers
Gerald (Jerry) Carter | 1 May 2009 13:32

Re: STatus update on Bugzilla.samba.org


Christian Perrier wrote:
> Quoting Gerald Carter (jerry <at> plainjoe.org):
>> -----BEGIN PGP SIGNED MESSAGE-----
>> Hash: SHA1
>>
>> Hey all,
>>
>> Big thanks to Deryck for quick fixes to bugzilla.samba.org.
>>
>>  * The "Too many connections" was the result of a bot.
>>    Fixed in robots.txt now.
> 
> Just in case: that bot is not a Debian one, right? 

Nope.  It was googlebot.  Just a misplaced robots.txt.

cheers, jerry
--
=====================================================================
Samba                                    ------- http://www.samba.org
Likewise Software          ---------  http://www.likewisesoftware.com
"What man is a man who does not make the world better?"      --Balian
Permalink | Reply | 0 comments |
headers
Stefan (metze) Metzmacher | 1 May 2009 16:05
Picon
Favicon

Re: [PATCH] Have the smbcli_session record the OS and Native LANMAN of the remote server

Hi Sam,

> * Amin Azez wrote, On 30/04/09 09:22:
>> The documents you mention encourage the style I used, and do not
>> discourage it, apart from tab-8 spacing which no-one follows.
>>   
> Perhaps everyone does follow that, it was careless reading on my part, I
> thought it said 8 spaces but it said NOT 8 spaces.
> 
> I don't think I misread anything else - here's hoping!

See 5604e8d614c938876b0a8cbc6f8c38262588f961 in master,
this patch only sets the strings on the final session setup.

metze
Permalink | Reply | 0 comments |
headers
Stefan (metze) Metzmacher | 1 May 2009 16:15
Picon
Favicon

Re: [PATCH] Have the smbcli_session record the OS and Native LANMAN of the remote server

Amin Azez schrieb:
> * Gerald Carter wrote, On 29/04/09 17:32:
>>> There's more than one established convention widely
>>> present in the code base.
>> Agreed.  But this should not really be a question of
>> personal style.  If it matches README.Coding and
>> prog_guide4.txt you should be safe.  If you follow
>> that and the patch is refused due to style reasons,
>> it is the project's fault and not yours.
> 
> I'm not picking fault here, or trying to argue; just pointing out how it
> looks from my position.
> 
> The documents you mention encourage the style I used, and do not
> discourage it, apart from tab-8 spacing which no-one follows.
> 
> The two documents you mention make recommendations that are not followed
> even in new code.
> 
> In short, they can't be trusted, it seems that they misled me and are
> somewhat ignored by everyone else.
> 
> 
> 
> prog_guide4.txt
> 
> I've read prog_guide4.txt numerous times before before and re-read it today.
> 
> It's still only "solicit[ing] comments from a few" and the only
> reference to the aspects of style we were discussing is a preference for
(Continue reading)

Permalink | Reply | 1 comment |
headers
Jelmer Vernooij | 1 May 2009 16:15
Picon
Favicon

Moving the docs from db2latex to dblatex?

At the moment we're using db2latex to generate LaTeX from the DocBook
sources that we later process to create PDFs and PostScript files. We
currently ship our own copy of db2latex with some workarounds for bugs
in it.

dblatex is a friendly fork of db2latex that is actually maintained
upstream, and allows us to remove a bunch of custom stylesheets with
workarounds and Makefile magic that takes care of intermediate build steps.

dblatex is packaged for all major distributions and is part of the
toolchain to build the PDF docs for various other projects, including
e.g. git (through asciidoc).

Migrating should be relatively easy, I'm working on a patch to do this.
Does anybody object to using dblatex instead?

Cheers,

Jelmer
Permalink | Reply | 3 comments |
headers
simo | 1 May 2009 16:41
Picon
Favicon

Re: [PATCH] Have the smbcli_session record the OS and Native LANMAN of the remote server

On Fri, 2009-05-01 at 16:15 +0200, Stefan (metze) Metzmacher wrote:
> We clearly need to combine, update and improve our coding guides.
> 
> At least for me it would make sense to have a rule to
> - do only one thing in one line.
> - don't assign variables within if statements
> - don't call functions as function parameters, use helper variables.
> 
>   functiona(functionb()) is bad
> 
> - prefer using
>   if (x == y) {
>       return;
>   }
>   instead of
>   if (x == y)
>       return;

+1, if most agree we should update the coding style guidelines.

Simo.

--

-- 
Simo Sorce
Samba Team GPL Compliance Officer <simo <at> samba.org>
Principal Software Engineer at Red Hat, Inc. <simo <at> redhat.com>
Permalink | Reply | 0 comments |

Gmane