Re: [PATCH] cygcheck: follow symbolic links

[Igor Peshansky <pechtcha at cs dot nyu dot edu>]

On Mon, 24 Jul 2006, Corinna Vinschen wrote:

> On Jul 21 20:52, Igor Peshansky wrote:
> > 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.
>
> The latest fax was about this change, so I think this should still be
> covered, shouldn't it?  Ping the guy nevertheless.  We should stay on
> the safe side in legal questions.
>
> I'd be happy to apply the patch, but it would be nice if you could tweak
> the formatting somewhat:
>
> > +  if (GetLastError () != NO_ERROR) display_error ("get_dword");
>
> The display_error call should be on its own line, as usual.  This
> happens multiple times in your patch.
>
> > +  if (is_exe (fh))
> > +    dll_info (path, fh, lvl, 1);
> > +  else if (is_symlink (fh))
> > +    display_error ("track_down: Huh?  Got a symlink!");
>
> Is that really the supposed message here?
>
> > +      printf (" - Not a DLL: magic number %x (%d) '%s'\n", magic, magic, (char *)&magic);
>
> Please split the printf so that it's not longer than 80 chars.
>
> > +      /* TODO: check for invalid path contents (see ../cygwin/path.cc:3313 */
>
> Since source code lines are most volatile, I'd not refer to a line number
> in another source code.  Just mention the function name. */
>
> > +      if (got != sizeof (buf) || memcmp (buf, SYMLINK_COOKIE, sizeof (buf)) != 0)
>
> Split the line, please.
>
> > +      if (SetFilePointer (fh, 0x4c + offset + 4, 0, FILE_BEGIN) == INVALID_SET_FILE_POINTER
>
> Same here.
>
> > +	{
> > +	  return false;
> > +	}
>
> I'd rather not have these one liners in curly brackets.  It's a bit
> irritating since sometimes you put them in curly brackets, sometimes you
> don't.
>
> The code looks ok, otherwise.

Looks like I've been remiss in following up on this, though I regenerated
the patch that same day.  Attached is the new version of the patch.  I
believe "the fax" (the new one) has been sent, but I've received no
notification of that, presumably because Corinna is not around...
	Igor
==============================================================================
ChangeLog:
2006-09-28  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