Bug #75193 for PAR-Packer: [PATCH] Fix build on Win32/VC++

Tue Feb 21 09:32:44 2012 shay [...] cpan.org - Ticket created

Subject:

[PATCH] Fix build on Win32/VC++

The current version (1.012) of PAR-Packer does not build on Win32 with VC++. There are two problems: myldr/boot.c attempts to include unistd.h which VC++ does not provide (MinGW/gcc does, but VC++ should include io.h for write() instead), and myldr/Makefile.PL wrongly uses $(CC) rather than $(LD) to link boot.exe. The attached patch fixes these problems. There is still a warning (not an error that stops the build) from myldr/mktmpdir.c which I'm not sure how best to solve: myldr\mktmpdir.c(45) : warning C4013: 'getuid' undefined; assuming extern returning int The problem is that Win32/VC++ doeesn't have a getuid(), hence the compiler's warning. The linker is happy, though, presumably because perl provides a dummy function in win32/win32.c: #define ROOT_UID ((uid_t)0) uid_t getuid(void) { return ROOT_UID; } but adding a declaration like extern uid_t getuid(void); to myldr/mktmpdir.h doesn't work at all.

Subject:

win32.patch

diff -ruN PAR-Packer-1.012.orig\myldr\boot.c PAR-Packer-1.012\myldr\boot.c --- PAR-Packer-1.012.orig\myldr\boot.c Mon Nov 14 17:13:56 2011 +++ PAR-Packer-1.012\myldr\boot.c Tue Feb 21 13:55:55 2012 @@ -1,6 +1,10 @@ #undef readdir +#if !defined(WIN32) || defined(__GNUC__) #include <unistd.h> +#else +#include <io.h> +#endif typedef struct my_chunk { diff -ruN PAR-Packer-1.012.orig\myldr\Makefile.PL PAR-Packer-1.012\myldr\Makefile.PL --- PAR-Packer-1.012.orig\myldr\Makefile.PL Mon Nov 28 15:06:36 2011 +++ PAR-Packer-1.012\myldr\Makefile.PL Tue Feb 21 14:03:07 2012 @@ -315,7 +315,7 @@ boot$o: \$(MKTMP_STUFF) $my_stuff $boot_exe: boot$o - \$(CC) boot$o \$(STATIC_LDFLAGS) $res $out$boot_exe_link + \$(LD) boot$o \$(STATIC_LDFLAGS) $res $out$boot_exe_link $mt_cmd my_par.c: $par_exe

Tue Feb 21 09:47:42 2012 RSCHUPP [...] cpan.org - Status changed from 'new' to 'open'

Tue Feb 21 09:47:44 2012 RSCHUPP [...] cpan.org - Taken

Tue Feb 21 10:10:31 2012 RSCHUPP [...] cpan.org - Correspondence added

On 2012-02-21 09:32:44, SHAY wrote: Show quoted text

> The current version (1.012) of PAR-Packer does not build on Win32 with > VC++. There are two problems: myldr/boot.c attempts to include unistd.h

Thanks for the patch, these changes were already in trunk. Show quoted text

> myldr\mktmpdir.c(45) : warning C4013: 'getuid' undefined; assuming > extern returning int

I wrapped the whole function calling getuid (isSafeDir) in #ifndef WIN32 as it's only called on *nix anyway. OTOH if someone could implement isSafeDir for Windows, I surely would appreciate that :) It's there to fix a CVE "PAR::Packer creates per-user directory with unsafe permissions". I'm not sure what it takes to create a "safe per-user directory" on Windows, i.e. where only the user (and privileged accounts) can create and modify files. If you want to test the current snapshot (esp. since you use the VC++ toolchain), there were some recent changes in the resource handling for Win32 executables http://svn.openfoundry.org/par/PAR-Packer/trunk Cheers, Roderich

Tue Feb 21 10:10:33 2012 RSCHUPP [...] cpan.org - Status changed from 'open' to 'patched'

Wed Feb 22 09:04:04 2012 SteveHay [...] planit.com - Correspondence added

Subject:	RE: [rt.cpan.org #75193] [PATCH] Fix build on Win32/VC++
Date:	Wed, 22 Feb 2012 14:03:00 -0000
To:	<bug-PAR-Packer [...] rt.cpan.org>, <par [...] perl.org>
From:	"Steve Hay" <SteveHay [...] planit.com>

Roderich Schupp via RT wrote on 2012-02-21: Show quoted text

> Tue Feb 21 10:10:31 2012: Request 75193 was acted upon. > Transaction: Correspondence added by RSCHUPP > Queue: PAR-Packer > Subject: [PATCH] Fix build on Win32/VC++ > Broken in: 1.012 > Severity: Important > Owner: RSCHUPP > Requestors: shay@cpan.org > Status: open > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=75193 > > > On 2012-02-21 09:32:44, SHAY wrote:

>> The current version (1.012) of PAR-Packer does not build on Win32 with >> VC++. There are two problems: myldr/boot.c attempts to include >> VC++unistd.h

> > Thanks for the patch, these changes were already in trunk. >

Oops, sorry. I should have checked there first. Show quoted text

>> myldr\mktmpdir.c(45) : warning C4013: 'getuid' undefined; assuming >> extern returning int

> > I wrapped the whole function calling getuid (isSafeDir) in #ifndef > WIN32 as it's only called on *nix anyway. > > OTOH if someone could implement isSafeDir for Windows, I surely would > appreciate that :) It's there to fix a CVE "PAR::Packer creates per- > user directory with unsafe permissions". > I'm not sure what it takes to create a "safe per-user directory" > on Windows, i.e. where only the user (and privileged accounts) can > create and modify files. > > If you want to test the current snapshot (esp. since you use the VC++ > toolchain), there were some recent changes in the resource handling for > Win32 executables > > http://svn.openfoundry.org/par/PAR-Packer/trunk >

Works fine for me, except that 20-pp.t still fails test 32 as before. Is that supposed to be fixed now? Do I need a newer Win32-Exe than I've got? I'm using 0.17. (The error is different now: it now complains about not being a valid Win32 executable, rather than complaining that its side-by-side configuration is incorret.)

Wed Feb 22 09:04:09 2012 The RT System itself - Status changed from 'patched' to 'open'

Wed Feb 22 16:35:26 2012 RSCHUPP [...] cpan.org - Correspondence added

On 2012-02-22 09:04:04, SteveHay@planit.com wrote: Show quoted text

> Works fine for > me, except that 20-pp.t still fails test 32 as before. Is that > supposed to be fixed now? Do I need a newer Win32-Exe than I've got? > I'm using 0.17.

That's the most recent version. Test 32 checks "pp --gui", right? Show quoted text

> (The error is different now: it now complains about > not being a valid Win32 executable, rather than complaining that its > side-by-side configuration is incorret.)

Yeah, I see that sometimes, too. And always with "pp --info ...". Somehow Win32::Exe seems to corrupt the executable - it's workings are black magic to me. There doesn't seem to exist a standard command on Windows to change icon or properties of an executable, the only MS sanctioned way seems to set them from a resource file when linking the executable. Cheers, Roderich