getpass regressions on Solaris/Illumos - 3.6 and master.

[Ira Cooper]

The regression is that people keep forgetting the header file that needs to
be included to make getpass work on Solaris and other non-linux OSes.

add_getpass.patch is almost right.  I missed 2 lines.  I've added them to
the attached patch.  (I believe it is me, being pedantic, but...)

As far as making it require getpassphrase: I don't think getpassphrase is
on Linux, is it?  And I'm not sure about the other platforms that may exist
with ok getpass but not a getpassphrase.

I'm ok with what is attached here. :)

-Ira

On Mon, Jan 30, 2012 at 8:57 PM, Jiri Sasek - Solaris Prague <
jiri.sasek at oracle.com> wrote:

> Hi Jeremy,
>
> I do not understand so much of this "gymnastics" here so I start to worry
>  :-)
>
> Some longer time ago I asked to replace getpass() which represents the
> original AT&T UNIX API (it has internal buffer for 8 chars!!! no more!!!
> more chars entered are ignored) by getpassphrase() which has also the
> internal buffer but 256chars long. Both calls are prototyped in
> /usr/include/stdlib.h  ...what kind of the regression it is? Is it
> introduced with the new waf build system?
>
> I do not understand full context of the proposed patches. But
> add_getpass.patch looks more like the changes I asked for a time ago. So I
> vote for add_getpass.patch.
>
> Best regards,
>
> Jiri
>
>
> On 01/31/12 02:09, Jeremy Allison wrote:
>
>> On Mon, Jan 30, 2012 at 10:27:39AM -0500, Ira Cooper wrote:
>>
>>> In looking at this issue in more detail:
>>>
>>> I've heard the argument: Putting all the header files in replace.h will
>>> increase build time.  So I did the experiment.  I put all the system/*
>>> include files in replace.h except for system/kerberos.h, because
>>> system/kerberos.h broke my build.  So far the difference doesn't look
>>> very
>>> big at all, at least on my machine!  Now, I'll admit my development
>>> machine
>>> is a really bad place to benchmark builds, so I'd like others to chime in
>>> with their results, if there is interest in this approach.  This patch is
>>> include_it_all.patch.
>>>
>>> (make -j64 of our autoconfed build; on my dev box.)
>>>
>>> Without the extra headers:
>>>
>>> real    0m47.438s
>>> user    8m38.416s
>>> sys     1m17.440s
>>>
>>> With them:
>>>
>>> real    0m49.108s
>>> user    8m55.999s
>>> sys     1m23.193s
>>>
>>> There is a second viable approach that metze pointed at, but this is NOT
>>> what he suggested directly.  Pull the getpass section of system/passwd.h
>>> into replace.h  Including the definition of rep_getpass.  If you do it
>>> without that definition, you'll break platforms without a good getpass
>>> and
>>> getpassphrase.  :(  This patch is add_getpass.patch.
>>>
>>> Diffs for both approaches are included.
>>>
>>> The add_getpass.patch is what I consider the "absolute minimum" to close
>>> this issue.  But I thought the first approach was interesting enough, and
>>> the result interesting enough that I've included both.  I'd like to see
>>> other developer's times on the "include_it_all.patch".  Please be careful
>>> of caching effects.  Build both ways a few times.
>>>
>> Ok, I like the add_getpass.patch as it seems smaller and cleaner to
>> me.
>>
>> Metze, or anyone else with an interest can you please comment on
>> this asap ?
>>
>> Otherwise I think I'll just add add_getpass.patch into master and
>> drive it through the 3.6.next process.
>>
>> Cheers,
>>
>>        Jeremy.
>>
>
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: add_getpass2.patch
Type: application/octet-stream
Size: 1037 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20120130/c66223f5/attachment.obj>