RE: svn commit: samba r11658 - in branches/SAMBA_3_0/source: .

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

RE: svn commit: samba r11658 - in branches/SAMBA_3_0/source: .

Green, Paul
> Jeremy Allison wrote:
|> Jerry Carter wrote:
|> Log:
|> Someone broke the initialization of the static modules by adding a
|> 'NTSTATUS' declaration before their call.
|> The compiler sees : { NTSTATUS fn_foo(); NT_STATUS fn_bar(); } as
|> *definitions: They need to be : { fn_foo(); fn_bar(); } Jeremy.
|
| Strangely enough this was only broken in SAMBA_3_0, not HEAD. Odd.
>
>That was Paul checkin.  Sorry Paul.

Yeah, I was going to reply with "That wasn't someone, that was me."  I
am not hiding. I did it. However, I most certainly tested this change
(one two platforms, VOS and SuSE Linux) and I can assure you that it is
a good change.  I can also assure you that it is a most necessary change
for gcc on VOS.

Oddly enough, it broke on the build farm machine that I myself run --
berks!!  This is a fairly slow machine (~300 MHz).  The log shows that
the build exceeded the time limit...

Would you mind if I restore the change?

Would you further mind if I raised the time limit so we don't get
bamboozled again?

Thanks
PG
--
Paul Green, Senior Technical Consultant,
Stratus Technologies, Maynard, MA USA
Voice: +1 978-461-7557; FAX: +1 978-461-3610
Reply | Threaded
Open this post in threaded view
|

Re: svn commit: samba r11658 - in branches/SAMBA_3_0/source: .

Gerald Carter-4
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Green, Paul wrote:

| Yeah, I was going to reply with "That wasn't someone,
| that was me."  I am not hiding. I did it. However, I
| most certainly tested this change (one two platforms,
| VOS and SuSE Linux) and I can assure you that it is
| a good change.  I can also assure you that it is
| a most necessary change for gcc on VOS.

I didn't notice any breakage either but I may not
have done a build from scratch between the two checkins.

| Oddly enough, it broke on the build farm machine that I
| myself run -- berks!!  This is a fairly slow machine
| (~300 MHz).  The log shows that the build exceeded
| the time limit...
|
| Would you mind if I restore the change?

I'll leave that up to Jeremy.

| Would you further mind if I raised the time limit
| so we don't get bamboozled again?

I don't normally get involved in the build farm.  Maybe
Andrew B., Vance (where is he these days?) or Tridge
have a feeling about it.






cheers, jerry
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.0 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org

iD8DBQFDdLBbIR7qMdg1EfYRAny/AKDz+DluCrUTheaBhtTHOMMyeOucQACfYZ+7
1lrGlNGldYTVwAI594uXn2Q=
=MKmn
-----END PGP SIGNATURE-----
Reply | Threaded
Open this post in threaded view
|

Re: svn commit: samba r11658 - in branches/SAMBA_3_0/source: .

Jeremy Allison
In reply to this post by Green, Paul
On Fri, Nov 11, 2005 at 08:48:43AM -0500, Green, Paul wrote:

> > Jeremy Allison wrote:
> |> Jerry Carter wrote:
> |> Log:
> |> Someone broke the initialization of the static modules by adding a
> |> 'NTSTATUS' declaration before their call.
> |> The compiler sees : { NTSTATUS fn_foo(); NT_STATUS fn_bar(); } as
> |> *definitions: They need to be : { fn_foo(); fn_bar(); } Jeremy.
> |
> | Strangely enough this was only broken in SAMBA_3_0, not HEAD. Odd.
> >
> >That was Paul checkin.  Sorry Paul.
>
> Yeah, I was going to reply with "That wasn't someone, that was me."  I
> am not hiding. I did it. However, I most certainly tested this change
> (one two platforms, VOS and SuSE Linux) and I can assure you that it is
> a good change.  I can also assure you that it is a most necessary change
> for gcc on VOS.
>
> Oddly enough, it broke on the build farm machine that I myself run --
> berks!!  This is a fairly slow machine (~300 MHz).  The log shows that
> the build exceeded the time limit...
>
> Would you mind if I restore the change?

Very much ! It breaks Samba completely on Linux (maybe with old compilers,
but still). This change is absolutely incorrect.

Jeremy.
Reply | Threaded
Open this post in threaded view
|

Re: svn commit: samba r11658 - in branches/SAMBA_3_0/source: .

Jeremy Allison
On Fri, Nov 11, 2005 at 09:01:21AM -0800, Jeremy Allison wrote:

> On Fri, Nov 11, 2005 at 08:48:43AM -0500, Green, Paul wrote:
> > > Jeremy Allison wrote:
> > |> Jerry Carter wrote:
> > |> Log:
> > |> Someone broke the initialization of the static modules by adding a
> > |> 'NTSTATUS' declaration before their call.
> > |> The compiler sees : { NTSTATUS fn_foo(); NT_STATUS fn_bar(); } as
> > |> *definitions: They need to be : { fn_foo(); fn_bar(); } Jeremy.
> > |
> > | Strangely enough this was only broken in SAMBA_3_0, not HEAD. Odd.
> > >
> > >That was Paul checkin.  Sorry Paul.
> >
> > Yeah, I was going to reply with "That wasn't someone, that was me."  I
> > am not hiding. I did it. However, I most certainly tested this change
> > (one two platforms, VOS and SuSE Linux) and I can assure you that it is
> > a good change.  I can also assure you that it is a most necessary change
> > for gcc on VOS.
> >
> > Oddly enough, it broke on the build farm machine that I myself run --
> > berks!!  This is a fairly slow machine (~300 MHz).  The log shows that
> > the build exceeded the time limit...
> >
> > Would you mind if I restore the change?
>
> Very much ! It breaks Samba completely on Linux (maybe with old compilers,
> but still). This change is absolutely incorrect.

The reason is this :

{ NTSTATUS fn_foo();  }

is not seen as a function call, it is seen as a declaration. This
means that none of the passdb modules get initialized. At all.

As you can imagine this breaks everything...

Now, if you replaced this with :

{ NTSTATUS foo_ret = fn_foo(); }

(when you modify foo_ret for each call so that you don't get duplicate
declarations) then that should work.

Jeremy.
Reply | Threaded
Open this post in threaded view
|

RE: svn commit: samba r11658 - in branches/SAMBA_3_0/source: .

Green, Paul
In reply to this post by Green, Paul
Jeremy Allison [mailto:[hidden email]]

> The reason is this :
>
> { NTSTATUS fn_foo();  }
>
> is not seen as a function call, it is seen as a declaration. This
> means that none of the passdb modules get initialized. At all.
>
> As you can imagine this breaks everything...
>
> Now, if you replaced this with :
>
> { NTSTATUS foo_ret = fn_foo(); }
>
> (when you modify foo_ret for each call so that you don't get duplicate
> declarations) then that should work.
>
> Jeremy.

Yes.  I confused the initialization and the declaration of the
functions.

I apologize profusely and twice.  Once for getting the analysis wrong
and once for getting the testing wrong.  I am embarrassed to return to
the Samba scene only to make such a fool of myself in public.

I'm working up a patch that creates a declaration string independently
of the initialization string.  Programs like lib/iconv.c can then be
changed to say

static_decl_charset;  /* expands to "extern NTSTATUS
charset_CP850_init(); ..." */

At the top, and continue to say

static_init_charset;  /* expands to "{ charset_CP850(); ... } */

When they are ready to invoke the initialization functions.    There are
6 such source programs, and 6 such variables.

I'll post the patch here, and **properly** test it before committing it.

PG
Reply | Threaded
Open this post in threaded view
|

Re: svn commit: samba r11658 - in branches/SAMBA_3_0/source: .

Jeremy Allison
On Fri, Nov 11, 2005 at 02:54:09PM -0500, Green, Paul wrote:
>
> Yes.  I confused the initialization and the declaration of the
> functions.
>
> I apologize profusely and twice.  Once for getting the analysis wrong
> and once for getting the testing wrong.  I am embarrassed to return to
> the Samba scene only to make such a fool of myself in public.

No problem Paul, don't beat yourself up too badly. I often bork
an entire release :-(. This is nothing as bad as that :-).

Thanks,

        Jeremy.
Reply | Threaded
Open this post in threaded view
|

Re: svn commit: samba r11658 - in branches/SAMBA_3_0/source: .

Andrew Tridgell-3
In reply to this post by Gerald Carter-4
Paul,

 > | Would you further mind if I raised the time limit
 > | so we don't get bamboozled again?

If you can tell me what timelimits you want, then I'll create a
berks.fn that sets the times. Or you can just commit one yourself in
the build_farm tree.

Cheers, Tridge
Reply | Threaded
Open this post in threaded view
|

Re: svn commit: samba r11658 - in branches/SAMBA_3_0/source: .

David Collier-Brown
In reply to this post by Green, Paul
Green, Paul wrote:
> Yes.  I confused the initialization and the declaration of the
> functions.
>
> I apologize profusely and twice.  Once for getting the analysis wrong
> and once for getting the testing wrong.  I am embarrassed to return to
> the Samba scene only to make such a fool of myself in public.

  Hey, don't feel bad.
  The C compiler crew at Honeywell made almost exactly the
same error.... I wrote a declaration and forgot the code,
and it generated code with an empty declaration. Which
core-dumped all over me (;-))

--dave
--
David Collier-Brown,      | Always do right. This will gratify
Sun Microsystems, Toronto | some people and astonish the rest
[hidden email]     |                      -- Mark Twain
(416) 263-5733 (x65733)   |
Reply | Threaded
Open this post in threaded view
|

Re: svn commit: samba r11658 - in branches/SAMBA_3_0/source: .

Vance Lankhaar
In reply to this post by Gerald Carter-4
> I don't normally get involved in the build farm.  Maybe
> Andrew B., Vance (where is he these days?) or Tridge
> have a feeling about it.

I, I'm ashamed to admit, uh, don't actually have Internet at home right now....

I'm still here, just a little slow in reading email / borrowing
Internet access from friends / etc.

Cheers,
Vance Lankhaar
Samba Team
Reply | Threaded
Open this post in threaded view
|

Re: svn commit: samba r11658 - in branches/SAMBA_3_0/source: .

Vance Lankhaar-2
In reply to this post by Andrew Tridgell-3
>  > | Would you further mind if I raised the time limit
>  > | so we don't get bamboozled again?
>
> If you can tell me what timelimits you want, then I'll create a
> berks.fn that sets the times. Or you can just commit one yourself in
> the build_farm tree.

Or ping me me with a timelimit, and I can set up berks.fns too ;-)

Cheers,
Vance Lankhaar
Samba Team