[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 2/3] forkpty, openpty: split functions into new modules

From: Eric Blake
Subject: Re: [PATCH 2/3] forkpty, openpty: split functions into new modules
Date: Thu, 18 Mar 2010 19:46:09 -0600
User-agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv: Gecko/20100301 Fedora/3.0.3-1.fc12 Lightning/1.0b1 Thunderbird/3.0.3

On 03/18/2010 07:35 PM, Bruno Haible wrote:
> Hi Eric,
> You requested a rapid review:

Well, supplying rapid comments saying 'please give me more time' are
also acceptable to slow me down; but I do appreciate the rapid review!

> Eric Blake wrote:
>> +  AC_SEARCH_LIBS([forkpty], [util libutil],
>> +    [if test "$ac_cv_search_forkpty" != "none required"; then
>> +       PTY_LIB="$ac_cv_search_forkpty"
>> +     fi])
> The second argument [util libutil] can be simplified to [util], no?
> Searching for liblibutil sounds useless.

Argh.  I was thinking <util.h> vs. <libutil.h>, but this is for -lutil
(vs. -lc).  You are correct; I'll simplify that.

>> +Files:
>> +m4/forkpty.m4
>> +
>> +Depends-on:
>> +pty
> I don't understand two things here:
>   - Why does 'forkpty' depend on 'openpty'?

I guess it doesn't have to.  It's just that for now, test-forkpty.c
tests both interfaces.

>   - Since forkpty.m4 is used for both 'forkpty' and 'openpty', it is asymetric
>     and a bit misleading to call it 'forkpty.m4'. Why not call it 'pty.m4'?

Sure; I named it forkpty.m4 to avoid confusion with the fact that I just
renamed pty.m4 -> pty_h.m4 in the previous patch.  But pty.m4 for both
forkpty and openpty makes sense.  I'll post a respin of the patches,
incorporating your comments, and restart the review clock.

Eric Blake   address@hidden    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

reply via email to

[Prev in Thread] Current Thread [Next in Thread]