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 Thu, 23 Feb 2006, Igor Peshansky wrote:

> On Thu, 23 Feb 2006, Corinna Vinschen wrote:
>
> > On Feb 22 13:55, Igor Peshansky wrote:
> > > On Fri, 17 Feb 2006, Igor Peshansky wrote:
> > > > On Fri, 17 Feb 2006, Corinna Vinschen wrote:
> > > > > On Feb 16 12:26, Igor Peshansky wrote:
> > > > > > On Thu, 16 Feb 2006, Corinna Vinschen wrote:
> > > > > > > - Most of your patch should go into path.cc so it can be
> > > > > > >   reused, for instance in strace.
> > > > > >
> > > > > > Agreed -- that's why I put that TODO in there. :-)  Should I
> > > > > > move it in the next iteration of the patch?
> > > > >
> > > > > Please move it now.  I don't think it's non-trivial enough to
> > > > > justify multiple iterations.
> > > >
> > > > Whoops.  Misspoke.  I meant "incarnation".  Never mind, I'll just
> > > > do it. :-)  Expect a new patch today.
> > >
> > > I guess "today" is a stretchable concept. :-)  In any case, here's a
> > > new patch.  Moving things into path.cc turned out to be indeed
> > > non-trivial, since the new functionality was using static functions
> > > in cygcheck.cc which now needed to be moved out into a separate
> > > file.  I don't expect this to be applied right away (hence no
> > > ChangeLog), but is this along the lines of what you were expecting?
> >
> > Yes, this looks generally ok to me.  I didn't *test* your patch, but
> > from the look of it, it seems fine, with one exception:
> >
> > I don't see a reason to introduce a new fileutil.cc file.  Please move
> > the functions into path.cc, add the extern declarations to path.h (so
> > you can drop them from cygcheck.cc), and revert the Makefile changes.
> > Then, together with a neat ChangeLog entry, we're pratically done :-)
>
> The problem is that most of the functions don't belong in path.cc (e.g.,
> display_error).  To do this properly, they will have to be rewritten in
> a more general manner, to return the appropriate error code so that the
> failure condition can be identified, and display_error should move back
> to cygcheck.cc (in particular, display_error prints "cygcheck" as part
> of the error).  That's why I chose to add all the functions to cygcheck
> first -- they are just not yet ready for consumption by other programs.
>
> Sigh.  I'd really like this to get in by 1.5.20, but time is limited, so
> that may not happen.  I'll see what I can do to separate the general and
> the specific, and split this functionality between cygcheck.cc and
> path.cc.
>
> > Thanks for doing this, btw.  I really appreciate it.
>
> Hey, I promised to look into it... :-)
> 	Igor

Wow.  Almost exactly 5 months.  This has got to be some kind of a record.

In any case, here's the latest incarnation, with get_word and get_dword
folded into path.cc, and display_error returned to cygcheck.cc, where it
belongs.  Tested reasonably well (with symlinks pointing to symlinks,
etc).  I'll let you judge the neatness of the ChangeLog entry.  If I'm
lucky, this might just get into 1.5.21[*].
	Igor
[*] Corinna, I'm guessing this is sufficiently different that you can't
accept it without "the fax" -- I'll keep pinging the guy who's holding
this up, but this message is also supposed to confirm that there is a
working patch, and the delay is simply bureaucratic.  Oh, the
frustration...  If you judge the changes from the previous incarnation
to not be significant, just go ahead and apply this, given the previous
approval.
==============================================================================
2006-07-21 Igor Peshansky <pechtcha@cs.nyu.edu>

	* cygcheck.cc (get_word, get_dword): Move to path.cc.
	(LINK_EXTENSION): New macro.
	(check_existence): New static function.
	(find_on_path): Check for symbolic links if asked.
	(dll_info): New error handling.
	(track_down): Only call dll_info() for executables, display
	an error for symlinks, and print magic number for others.
	(find_app_on_path): New static function.
	(cygcheck, dump_sysinfo): Call find_app_on_path() instead of
	find_on_path().
	* path.cc (cmp_shortcut_header): New static function.
	(get_word, get_dword): Moved from cygcheck.cc.
	(EXE_MAGIC, SHORTCUT_MAGIC, SYMLINK_COOKIE, SYMLINK_MAGIC): New
	macros.
	(is_exe, is_symlink, readlink): New functions.
	* path.h (is_exe, is_symlink, readlink): Declare.
	(get_word, get_dword): Ditto.

-- 
				http://cs.nyu.edu/~pechtcha/
      |\      _,,,---,,_	    pechtcha@cs.nyu.edu | igor@watson.ibm.com
ZZZzz /,`.-'`'    -.  ;-;;,_		Igor Peshansky, Ph.D. (name changed!)
     |,4-  ) )-,_. ,\ (  `'-'		old name: Igor Pechtchanski
    '---''(_/--'  `-'\_) fL	a.k.a JaguaR-R-R-r-r-r-.-.-.  Meow!

"Las! je suis sot... -Mais non, tu ne l'es pas, puisque tu t'en rends compte."
"But no -- you are no fool; you call yourself a fool, there's proof enough in
that!" -- Rostand, "Cyrano de Bergerac"

Attachment: cygcheck-follow-symlinks.patch
Description: Text document


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