[perl #37303] [PATCH] Relaxing parrot dependency on parrot_config

----- Forwarded message from Nick Glencross <nick.gl...@gmail.com> -----

From: Nick Glencross <nick.gl...@gmail.com>
Reply-To: Nick Glencross <nick.gl...@gmail.com>
Date: Wed, 28 Sep 2005 22:07:15 +0100
To: perl6-i...@perl.org
Subject: Relaxing parrot dependency on parrot_config

Guys,

I've been wanting to relax the dependency that parrot's core has on
parrot_config. As things stand at the moment, src/global_setup.c makes
a call to parrot_get_config which is linked into the executable itself
by selecting either null_config.o, parrot_config.o or
install_config.o.

This works pretty well on most platforms so far, but is a major
headache for building on cygwin (and Windows?) and perhaps some future
platforms. Why? Well, some platforms have a limitation whereby a
Shared Library or DLL cannot access symbolic information in the
executable that loads them. This will affect us more as parrot is used
as a library, or we try to get dynclasses working on awkward
platforms.

What I've done is add a function to optionally register the config
string -- That way the call is exclusively from the executable into
parrot only. The contents of the config *.c files is slightly changed,
and the function parrot_get_config has been moved into a new file
src/config.c (better names gladly accepted!)

Since registering the config string is optional, utilities such as
pbc_merge and pdb no longer need to link with null_config.o.

At the moment this patch is for review as I'm expecting some good
advise before it hopefully gets accepted.

Cheers,

Nick

----- End forwarded message -----

It didn't make it in because I dropped the ball. Thanks for
resubmitting.

Cheers,

-J

--

I'm guessing that this change in src/embed.c wasn't intended to be included in the patch:

- mmap(0, program_size, PROT_READ, MAP_SHARED, fd, (off_t)0);
+ mmap(0, program_size, PROT_READ, MAP_PRIVATE, fd, (off_t)0);

???

-J

--

I also think that it would also be cleaner to move the lines

const char* parrot_config_ptr;
unsigned int parrot_config_size;

from library.h into the parrot harness itself.

All things considered, I think it's best if I repost the patch this
evening (UK time) and fix my botch.

Cheers,

Nick

* Removed the mmap nonsense which was sent by accident

* Renamed config.c to config_string.c to make it less generic

* Moved a couple externs from a core parrot library into the main
executable, where they sit better

Thanks for considering this patch,

Nick

On 10/18/05, Joshua Hoblitt via RT <parrotbug...@parrotcode.org> wrote:
> Chip,
>
> Looks like a design call.

I've now applied it locally, but there are 2 problems:
- small C99-ism (decl of interp in imcc/main.c:main()) and worse:
- segfault:
./miniparrot config_lib.pasm > runtime/parrot/include/config.fpmc
make: *** [runtime/parrot/include/config.fpmc] Error 139

Please check the patch, thanks.

> Nick

leo

It took me a moment to see what what you meant by the first point, but
yes, I've been a twit and put the Parrot_set_config_string line too
early in the function!

As for the second more serious issue, that's very odd! I assume that
you've tried building from scratch... (in particular no old
src/null_config.c) Also, what platform is this on?

Thanks.

[No point resubmitting the patch to fix the first point until we have an
explanation for the second, and after your nod of approval]

Nick

>
> Leopold Toetsch wrote:

> As for the second more serious issue, that's very odd! I assume that

I've done 'make realclean' ... Platform is x86 linux.

> Nick

leo

> I'm not sure that the patch made it into RT. Here it is again, with a

> src/config.c will need to be 'svn add'ed when applying the patch, and
> Configure rerun to recreate the top-level Makefile.

Thanks! I'll check in this patch in a little bit. Meanwhile I want to
comment on a few small nits that I'll clean up before I apply it.

+
+ print << "EOF";
+const char* parrot_config_ptr = 0;

Per coding standards, the pointer star needs to go on the variable name.

+ printf "0x%02x", ord($_);
+ ++$i;
+ print ', ', if ($i < scalar(@c));
+ print "\n " unless $i % 8;

The scalar() here is unnecessary. No big deal.

Index: src/config.c
===================================================================
--- src/config.c (revision 0)
+++ src/config.c (revision 0)
@@ -0,0 +1,63 @@
+/*
+ Copyright: 2005 The Perl Foundation. All Rights Reserved.
+ $Id$

I'll change the date to 2007.

+static const char *parrot_config_private_ptr = NULL;
+static unsigned int parrot_config_private_size = 0;

Eventually it might be better to move these variables into the parent
interpreter. For now, I don't think anyone's embedding multiple Parrots into
a single program.

+parrot_get_config_string(Interp* interpreter)
+{
+ if (!parrot_config_private_ptr)
+ return NULL;
+
+ return string_from_const_cstring(interpreter,
+ parrot_config_private_ptr,
+ parrot_config_private_size);
+}

I don't remember what the memory allocation characteristics are of
string_from_const_cstring(), so these functions may need comments
saying "It's your responsibility to free this string."

Index: src/pmc_freeze.c
===================================================================
--- src/pmc_freeze.c (revision 9273)
+++ src/pmc_freeze.c (working copy)
@@ -712,7 +712,7 @@
else {
if (string_length(interpreter, s) < PACKFILE_HEADER_BYTES) {
real_exception(interpreter, NULL, E_IOError,
- "bad string too thaw");
+ "bad string to thaw");
}
mem_sys_memcopy(pf->header, s->strstart, PACKFILE_HEADER_BYTES);
PackFile_assign_transforms(pf);

This looks like part of a separate patch.

===================================================================
--- imcc/main.c (revision 9273)
+++ imcc/main.c (working copy)
@@ -459,6 +459,8 @@
char *sourcefile;
char *output;

+ parrot_set_config_string(parrot_config_ptr,parrot_config_size);
+
Interp *interp = Parrot_new(NULL);

The new line should come after the variable declarations, per my understanding
of C89. This is difficult to remember. Stupid C89.

Good patch!

-- c