This is the mail archive of the cygwin-patches mailing list for the Cygwin project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

RE: [PATCH] cygcheck: follow symbolic links


On 17 February 2006 14:05, Igor Peshansky wrote:

> On Fri, 17 Feb 2006, Dave Korn wrote:
> 
>> On 16 February 2006 17:27, Igor Peshansky wrote:
>> 
>>> On Thu, 16 Feb 2006, Corinna Vinschen wrote:
>> 
>>>> - Couldn't you just reuse the readlink implementation in
>>>>   ../cygwin/path.cc as is, to avoid having to different implementations?
>>> 
>>> Umm, most of that code is very general purpose, and has too much extra
>>> stuff in it.
>> 
>>   I think you may have misoptimised for speed rather than
>> maintainability. Cygcheck isn't something that people expect to run a
>> million times per second in an inner loop.
> 
> No, but I thought ease of understanding implied maintainability...

  Yes, but cutting and pasting the same code into a duplicated location
doesn't make it any clearer, and in fact it impacts both maintainability _and_
clarity when it starts to diverge and someone come to look at it six months
later and wonders "Why has this functionality been implemented twice and why
are the two versions slightly different and which one is meant to be the right
way of doing it"

> Besides, I'm sure binutils, for one, has the code for reading chunks of
> application code and finding the DLL dependencies -- why aren't we reusing
> that?  The answer: too much work to extract the needed bits in the form
> that would be usable in both places.

  hmmm... maybe we should think about that....


>>>  I basically used part of it (symlink_info::check_shortcut)
>>> for my implementation.  I wanted something lightweight and easy to
>>> understand
>> 
>>   Perhaps you could have just exported it (or a convenient interface to
>> it) instead?
> 
> Ahem.  You are forgetting that cygcheck is not a Cygwin program, so we
> can't introduce a dependency on cygwin1.dll.  We'd have to create an
> independent (static?) library that both cygcheck and Cygwin depended on...

  Not necessarily.  We could unify it so they both build from the same one
single shared copy of path.cc instead of having the real deal in winsup/cygwin
and a stripped down copy that has to be manually synced every now and again in
winsup/utils... of course, that's really getting fairly sidetracked from the
issue you were trying to directly deal with.  But longterm it would be good if
they both got their code from the same place.

  Hey, does anyone know off the top of their heads of any other chunks of code
shared like this between cygwin and utils?  I could make it a sort-of long
term code tidyup project to try and get better sharing and reuse...


    cheers,
      DaveK
-- 
Can't think of a witty .sigline today....


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]