Fwd: Small patches for const/shared

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

Fwd: Small patches for const/shared

Martin Hedenfalk-4
> From: Jens Rehsack <[hidden email]>

> To: [hidden email]
> Subject: Fwd: Small patches for const/shared
>
> Hi,
>
> I think it's not a good idea to have a closed devel-mailing
> list. I do not intend to increase my engagement in OSS
> to one more library and collect mailing lists. I'm already
> subscribed to about 20 lists, I can't read them all when
> I add more.
Of course, you're absolutely right. It's rather silly to require subscription for this mailing list.

But it was closed out of frustration over the amount of spam it received. Unless Savannah has started using greylisting, however, I'm afraid it will stay that way.

I'll forward you email to the list. Thank you.

        -martin


> Jens
>
> ---------- Forwarded message ----------
> From:  <[hidden email]>
> Date: 2010/10/7
> Subject: Small patches for const/shared
> To: [hidden email]
>
>
> You are not allowed to post to this mailing list, and your message has
> been automatically rejected.  If you think that your messages are
> being rejected in error, contact the mailing list owner at
> [hidden email].
>
>
>
> ---------- Weitergeleitete Nachricht ----------
> From: Jens Rehsack <[hidden email]>
> To: [hidden email]
> Date: Thu, 7 Oct 2010 08:36:39 +0200
> Subject: Small patches for const/shared
> Hi all,
>
> first: thanks for libConfuse. It saves us a lot of work.
>
> Nevertheless I have two small patches (the const-patch probably could
> be extended to more functions).
> Furthermore, it doesn't solve the:
>
> config.cpp:46: warning: deprecated conversion from string constant to ‘char*’
> config.cpp:46: warning: deprecated conversion from string constant to ‘char*’
> config.cpp:54: warning: deprecated conversion from string constant to ‘char*’
> config.cpp:54: warning: deprecated conversion from string constant to ‘char*’
> ...
>
> It wasn't enought/didn't work to make all cfg_opt_t instances const like
> const cfg_opt_t foot_opts[] = {...};
> When I have some time after finishing the PoC, I'll take a deeper look.
>
> The second patch allows users to enable libconfuse' shared library target.
> I don't see any reason to prevent it at all. Linking compatibility can be solved
> with appropriate library version numbers
> (http://www.lrde.epita.fr/~adl/autotools.html).
>
> Best regards,
> Jens

>


_______________________________________________
Confuse-devel mailing list
[hidden email]
http://lists.nongnu.org/mailman/listinfo/confuse-devel

const.patch (2K) Download Attachment
allow-shared.patch (313 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Fwd: Small patches for const/shared

ohnobinki
> > ---------- Weitergeleitete Nachricht ----------

> > From: Jens Rehsack <[hidden email]>
> > To: [hidden email]
> > Date: Thu, 7 Oct 2010 08:36:39 +0200
> > Subject: Small patches for const/shared
> > Hi all,
> >
> > first: thanks for libConfuse. It saves us a lot of work.
> >
> > Nevertheless I have two small patches (the const-patch probably could
> > be extended to more functions).
> > Furthermore, it doesn't solve the:
> >
> > config.cpp:46: warning: deprecated conversion from string constant to ?char*?
> > config.cpp:46: warning: deprecated conversion from string constant to ?char*?
> > config.cpp:54: warning: deprecated conversion from string constant to ?char*?
> > config.cpp:54: warning: deprecated conversion from string constant to ?char*?
> > ...
> >
> > It wasn't enought/didn't work to make all cfg_opt_t instances const like
> > const cfg_opt_t foot_opts[] = {...};
> > When I have some time after finishing the PoC, I'll take a deeper look.
> >
I think const-correctness would be useful in any case. I haven't
looked closely enough at the implications of this patch yet, however
;-).

> > The second patch allows users to enable libconfuse' shared library target.
> > I don't see any reason to prevent it at all. Linking compatibility can be solved
> > with appropriate library version numbers
> > (http://www.lrde.epita.fr/~adl/autotools.html).

Even with AC_DISABLE_SHARED, one may specify --enable-shared to
./configure to get a shared libconfuse library. Thus, this patch isn't
necessary for users or distributions to compile shared libraries for
libconfuse. (for example, Gentoo passes --enable-shared to confuse's
./configure script).

However, I also would like to see shared objects enabled by default as
well. And, yes, if the ABI changes between releases, I hope to make
sure that the libtool's -version-info flag is set properly. However, I
think that moving a function's argument from being non-const to being
const doesn't affect the ABI... thus, the default version flag
(-version-info 0:0:0) should be sufficient when only accounting for
the changes in the first patch (and that's what libtool's been
assuming so far anyways).

If it's fine with others, I'll remove AC_DISABLE_SHARED and put in an
empty -version-info flag tonight.

> >
> > Best regards,
> > Jens


--
binki

Look out for missing apostrophes!

_______________________________________________
Confuse-devel mailing list
[hidden email]
http://lists.nongnu.org/mailman/listinfo/confuse-devel

attachment0 (205 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Fwd: Small patches for const/shared

ohnobinki
On Fri, Oct 08, 2010 at 04:18:55PM +0000, Jens Rehsack wrote:

> >>> The second patch allows users to enable libconfuse' shared library target.
> >>> I don't see any reason to prevent it at all. Linking compatibility can be solved
> >>> with appropriate library version numbers
> >>> (http://www.lrde.epita.fr/~adl/autotools.html).
> >
> > Even with AC_DISABLE_SHARED, one may specify --enable-shared to
> > ./configure to get a shared libconfuse library. Thus, this patch isn't
> > necessary for users or distributions to compile shared libraries for
> > libconfuse. (for example, Gentoo passes --enable-shared to confuse's
> > ./configure script).
>
> I tried that first (Ubuntu 10.04 LTS), and it didn't work.
I also just tried that on ubunto-10.04LTS:

$ http://savannah.nongnu.org/download/confuse/confuse-2.7.tar.gz
$ tar -xvzf confuse-2.7.tar.gz
$ cd confuse-2.7
$ ./configure --prefix=/usr --enable-shared
$ mkdir _destdir
$ make DESTDIR="${PWD}"/_destdir install -j
$ find _destdir -name '*.so'
_destdir/usr/lib/libconfuse.so

binki@csx:~/confuse-2.7$ cat /etc/*release*
DISTRIB_ID=Ubuntu
DISTRIB_RELEASE=10.04
DISTRIB_CODENAME=lucid
DISTRIB_DESCRIPTION="Ubuntu 10.04.1 LTS"


I get a shared library with no problem ;-).

> > However, I also would like to see shared objects enabled by default as
> > well. And, yes, if the ABI changes between releases, I hope to make
> > sure that the libtool's -version-info flag is set properly. However, I
> > think that moving a function's argument from being non-const to being
> > const doesn't affect the ABI...
>
> Not a C ABI ;)
> A C++ ABI would be affected.

If my understanding is correct, the C and C++ ABI are identical for
confuse. Nothing special is done in confuse.h for C++ except the
normal extern "C". A C++ program would be accessing and using the same
symbols as a C program.

The idea that ABI changes when moving from ``int a(const char *b);''
to ``int a(char *b);'' is that in the first case, the compiler and
programmer will know that the string passed as argument b will not be
modified. Thus, certain optimizations could be made such that a
program compiled against the ``int a(const char *b)'' definition would
be incompatible after the system library moved to having a() edit the
string b. But the reverse change should not encounter this problem.

--
binki

Look out for missing apostrophes!

_______________________________________________
Confuse-devel mailing list
[hidden email]
http://lists.nongnu.org/mailman/listinfo/confuse-devel

attachment0 (205 bytes) Download Attachment