<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.0 TRANSITIONAL//EN">
<HTML>
<HEAD>
<META HTTP-EQUIV="Content-Type" CONTENT="text/html; CHARSET=UTF-8">
<META NAME="GENERATOR" CONTENT="GtkHTML/3.3.2">
</HEAD>
<BODY>
MIsha,<BR>
<BR>
You asked for my feedback on Markus' recent patches. Here it is.<BR>
<BR>
On Tue, 2005-04-19 at 23:01 -0500, Misha Brukman wrote:
<BLOCKQUOTE TYPE=CITE>
<PRE>
<FONT COLOR="#000000">On Tue, Apr 19, 2005 at 07:01:40AM +0200, Markus F.X.J. Oberhumer wrote:</FONT>
<FONT COLOR="#000000">> While trying to hunt down a codegen bug (not yet found) ...</FONT>
<FONT COLOR="#000000">Have you considered using bugpoint for your codegen debugging needs?</FONT>
<FONT COLOR="#000000"><A HREF="http://llvm.cs.uiuc.edu/docs/Bugpoint.html#codegendebug">http://llvm.cs.uiuc.edu/docs/Bugpoint.html#codegendebug</A></FONT>
<FONT COLOR="#000000">> I've collected some small patches you might find useful.</FONT>
<FONT COLOR="#000000">Sweet!</FONT>
<FONT COLOR="#000000"> </FONT>
<FONT COLOR="#000000">> Please review and apply as you see fit.</FONT>
<FONT COLOR="#000000">I've applied your gccld patch. </FONT>
</PRE>
</BLOCKQUOTE>
<BR>
I concur.<BR>
<BR>
<BLOCKQUOTE TYPE=CITE>
<PRE>
<FONT COLOR="#000000">For the isExecutable patch, you check to see if it's a file first before</FONT>
<FONT COLOR="#000000">calling access(), but that precludes that function being used on a</FONT>
<FONT COLOR="#000000">directory (which is a valid sys::Path object and could be queried for</FONT>
<FONT COLOR="#000000">being executable), so I think it's unnecessary. </FONT>
</PRE>
</BLOCKQUOTE>
<BR>
The *original* intent of this method was to check for executable programs, not whether a directory is searchable. If we want that capability we should add a method named "searchable". Some operating systems don't have that concept and we should make the interface functions on Path address only one concept at a time. So, given that the "executable" method is verifying that the path points to a program that could be executed by the calling process, Markus patch is reasonable (needed, even).<BR>
<BR>
<BLOCKQUOTE TYPE=CITE>
<PRE>
<FONT COLOR="#000000">I also applied your</FONT>
<FONT COLOR="#000000">patch to ignore dangling symlinks.</FONT>
</PRE>
</BLOCKQUOTE>
<BR>
That part of the Path.h patch is similar to a previous one we committed and I concur.<BR>
<BR>
<BLOCKQUOTE TYPE=CITE>
<PRE>
<FONT COLOR="#000000">Finally, I applied your patch to ArchiveWriter with minor modifications</FONT>
<FONT COLOR="#000000">-- please see llvm-commits or the web archive.</FONT>
</PRE>
</BLOCKQUOTE>
<BR>
I concur.<BR>
<BR>
Reid.
</BODY>
</HTML>