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 ebl...@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
