[Bug 16352] Unimplemented Function in Scan-Build

Anton Yartsev anton.yartsev at gmail.com
Thu Jul 18 02:52:26 PDT 2013


On 03.07.2013 20:34, Jordan Rose wrote:
> Comments on patch (which probably should have gone to cfe-commits or Phabricator):
Attached the updated patch to http://llvm-reviews.chandlerc.com/D703

>
> +# portability: convert POSIX cygwin path to MS-DOS style path
>
> Please start comments with a capital letter.
>
>
> +  $PortablePath =~ s/^\/cygdrive\/(\w)/$1:/o if ($^O =~ /cygwin/);
>
> The user is free to modify the mount path for /cygdrive/, but I guess that's okay. What I'm more confused about is how programs running inside Cygwin are supposed to use Windows-ish paths (still with forward slashes) to run executables. But I don't have a Windows machine available, so I'll trust you know what's right here. Please do try to hand-test all the code paths affected by this if you haven't already.
Tested with pure cygwin, windows paths are unwillingly accepted, with 
warnings; cygwin docs says "Using native Win32 paths in Cygwin, while 
possible, is generally inadvisable."
Decided to remove path conversion for now. This enhancement was added 
for particular cases when cygwin is used together with other ports 
(MinGW + perl from Cygwin + separate Python in my case).

>
>
> +# portability: getpwuid is not implemented for Win32 (see Perl language reference, perlport)
> +my $UserName = ($^O =~/MSWin32/) ? HtmlEscape(getlogin || 'unknown')
> +                                 : HtmlEscape(getpwuid($<) || 'unknown');
>
> How about just getlogin() || getpwuid($<) || 'unknown'? That seems to be fairly idiomatic in a quick search online.
>
> (This is only used for metadata in the generated output, so it's not even that important.)
>
>
> +print "NewDir = ".$NewDir."\n";
>
> Stray debug print?
>
>
> +# portability: use less strict but portable check -e (file exists) instead of
> +# non-portable -x (file is executable). On some windows ports -x just checks
> +# file extension to determine if a file is executable (see Perl language reference, perlport)
>
> Capital letter, and we should also follow the 80-column limit going forward.
>


-- 
Anton




More information about the cfe-commits mailing list