FAQ Search Today's Posts Mark Forums Read
» Video Reviews

» Linux Archive

Linux-archive is a website aiming to archive linux email lists and to make them easily accessible for linux users/developers.


» Sponsor

» Partners

» Sponsor

Go Back   Linux Archive > ArchLinux > ArchLinux Pacman Development

 
 
LinkBack Thread Tools
 
Old 07-26-2012, 08:23 PM
Paul Barbu Gheorghe
 
Default removed duplicate macros SYMEXPORT and SYMHIDDEN from alpm_list.c, using them from util.h

Signed-off-by: Barbu Paul - Gheorghe <barbu.paul.gheorghe@gmail.com>
---
lib/libalpm/alpm_list.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/lib/libalpm/alpm_list.c b/lib/libalpm/alpm_list.c
index 39eded1..b9e7cba 100644
--- a/lib/libalpm/alpm_list.c
+++ b/lib/libalpm/alpm_list.c
@@ -21,13 +21,11 @@
#include <stdlib.h>
#include <string.h>
+#include "util.h"
+
/* libalpm */
#include "alpm_list.h"
-/* check exported library symbols with: nm -C -D <lib> */
-#define SYMEXPORT __attribute__((visibility("default")))
-#define SYMHIDDEN __attribute__((visibility("internal")))
-
/**
* @addtogroup alpm_list List Functions
* @brief Functions to manipulate alpm_list_t lists.
--
Barbu Paul - Gheorghe
Common sense is not so common - Voltaire
Visit My GitHub profile to see my open-source projects - https://github.com/paullik
 
Old 07-26-2012, 08:31 PM
Barbu Paul Gheorghe
 
Default removed duplicate macros SYMEXPORT and SYMHIDDEN from alpm_list.c, using them from util.h

Sorry for the mess with the empty attachment, I still have to learn how to
properly use git send-email and git imap-send, this is the first patch I ever
submitted this way.



Have a nice day.

--
Barbu Paul - Gheorghe
Common sense is not so common - Voltaire
Visit My GitHub profile to see my open-source projects - https://github.com/paullik
 
Old 07-26-2012, 08:51 PM
Dan McGee
 
Default removed duplicate macros SYMEXPORT and SYMHIDDEN from alpm_list.c, using them from util.h

On Thu, Jul 26, 2012 at 3:23 PM, Paul Barbu Gheorghe
<barbu.paul.gheorghe@gmail.com> wrote:
> Signed-off-by: Barbu Paul - Gheorghe <barbu.paul.gheorghe@gmail.com>
> ---
> lib/libalpm/alpm_list.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/lib/libalpm/alpm_list.c b/lib/libalpm/alpm_list.c
> index 39eded1..b9e7cba 100644
> --- a/lib/libalpm/alpm_list.c
> +++ b/lib/libalpm/alpm_list.c
> @@ -21,13 +21,11 @@
> #include <stdlib.h>
> #include <string.h>
> +#include "util.h"
> +
> /* libalpm */
> #include "alpm_list.h"
> -/* check exported library symbols with: nm -C -D <lib> */
> -#define SYMEXPORT __attribute__((visibility("default")))
> -#define SYMHIDDEN __attribute__((visibility("internal")))
> -

The reasoning behind this is alpm_list.c/h is a completely standalone
set of files, so we don't link back to anything in the rest of alpm.
With that said, the duplicates were quite on purpose and I'm not
inclined to apply this patch.

-Dan
 
Old 07-26-2012, 08:59 PM
Dave Reisner
 
Default removed duplicate macros SYMEXPORT and SYMHIDDEN from alpm_list.c, using them from util.h

On Thu, Jul 26, 2012 at 03:51:20PM -0500, Dan McGee wrote:
> On Thu, Jul 26, 2012 at 3:23 PM, Paul Barbu Gheorghe
> <barbu.paul.gheorghe@gmail.com> wrote:
> > Signed-off-by: Barbu Paul - Gheorghe <barbu.paul.gheorghe@gmail.com>
> > ---
> > lib/libalpm/alpm_list.c | 6 ++----
> > 1 file changed, 2 insertions(+), 4 deletions(-)
> >
> > diff --git a/lib/libalpm/alpm_list.c b/lib/libalpm/alpm_list.c
> > index 39eded1..b9e7cba 100644
> > --- a/lib/libalpm/alpm_list.c
> > +++ b/lib/libalpm/alpm_list.c
> > @@ -21,13 +21,11 @@
> > #include <stdlib.h>
> > #include <string.h>
> > +#include "util.h"
> > +
> > /* libalpm */
> > #include "alpm_list.h"
> > -/* check exported library symbols with: nm -C -D <lib> */
> > -#define SYMEXPORT __attribute__((visibility("default")))
> > -#define SYMHIDDEN __attribute__((visibility("internal")))
> > -
>
> The reasoning behind this is alpm_list.c/h is a completely standalone
> set of files, so we don't link back to anything in the rest of alpm.
> With that said, the duplicates were quite on purpose and I'm not
> inclined to apply this patch.
>
> -Dan
>

Hmmm, so this is totally non-obvious looking at the code and at history.
Why do we care about keeping alpm_list as an island? It's not treated
any differently in the buildsystem, and both alpm.h and alpm_list.h
expose symbols in a singular shared object.

If we aren't going to merge this, we should at least document the
duplication.

d
 
Old 07-27-2012, 10:12 AM
Barbu Paul Gheorghe
 
Default removed duplicate macros SYMEXPORT and SYMHIDDEN from alpm_list.c, using them from util.h

On 07/26/2012 11:51 PM, Dan McGee wrote:

The reasoning behind this is alpm_list.c/h is a completely standalone
set of files, so we don't link back to anything in the rest of alpm.


Why this decision?

I mean, there should be a good reson in order to duplicate code...

I'm trying to understand the things here, don't get me wrong.


--
Barbu Paul - Gheorghe
Common sense is not so common - Voltaire
Visit My GitHub profile to see my open-source projects - https://github.com/paullik
 
Old 07-30-2012, 08:05 PM
Barbu Paul - Gheorghe
 
Default removed duplicate macros SYMEXPORT and SYMHIDDEN from alpm_list.c, using them from util.h

Did anyone get 5 mins. to write an answer for my question?

Why isn't desirable to link alpm_list.{c,h} back to the rest of alpm?

--
Barbu Paul - Gheorghe
Common sense is not so common - Voltaire
Visit My GitHub profile to see my open-source projects - https://github.com/paullik
 
Old 07-30-2012, 11:12 PM
Allan McRae
 
Default removed duplicate macros SYMEXPORT and SYMHIDDEN from alpm_list.c, using them from util.h

On 27/07/12 06:59, Dave Reisner wrote:
> On Thu, Jul 26, 2012 at 03:51:20PM -0500, Dan McGee wrote:
>> On Thu, Jul 26, 2012 at 3:23 PM, Paul Barbu Gheorghe
>> <barbu.paul.gheorghe@gmail.com> wrote:
>>> Signed-off-by: Barbu Paul - Gheorghe <barbu.paul.gheorghe@gmail.com>
>>> ---
>>> lib/libalpm/alpm_list.c | 6 ++----
>>> 1 file changed, 2 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/lib/libalpm/alpm_list.c b/lib/libalpm/alpm_list.c
>>> index 39eded1..b9e7cba 100644
>>> --- a/lib/libalpm/alpm_list.c
>>> +++ b/lib/libalpm/alpm_list.c
>>> @@ -21,13 +21,11 @@
>>> #include <stdlib.h>
>>> #include <string.h>
>>> +#include "util.h"
>>> +
>>> /* libalpm */
>>> #include "alpm_list.h"
>>> -/* check exported library symbols with: nm -C -D <lib> */
>>> -#define SYMEXPORT __attribute__((visibility("default")))
>>> -#define SYMHIDDEN __attribute__((visibility("internal")))
>>> -
>>
>> The reasoning behind this is alpm_list.c/h is a completely standalone
>> set of files, so we don't link back to anything in the rest of alpm.
>> With that said, the duplicates were quite on purpose and I'm not
>> inclined to apply this patch.
>>
>> -Dan
>>
>
> Hmmm, so this is totally non-obvious looking at the code and at history.
> Why do we care about keeping alpm_list as an island? It's not treated
> any differently in the buildsystem, and both alpm.h and alpm_list.h
> expose symbols in a singular shared object.
>
> If we aren't going to merge this, we should at least document the
> duplication.

I believe the idea was to be able to just use alpm_list.{h,c} in another
project without linking to libalpm.

Personally, I see no need to keep that separation. There are more
general list implementations widely available and I have never seen
another project use this.

Allan
 
Old 07-31-2012, 06:04 AM
Barbu Paul - Gheorghe
 
Default removed duplicate macros SYMEXPORT and SYMHIDDEN from alpm_list.c, using them from util.h

On 07/31/2012 02:12 AM, Allan McRae wrote:

I believe the idea was to be able to just use alpm_list.{h,c} in another
project without linking to libalpm.

Personally, I see no need to keep that separation. There are more
general list implementations widely available and I have never seen
another project use this.


OK, I get it.

So it's up to you guys if you merge it, just let me know when you take a decision.

--
Barbu Paul - Gheorghe
Common sense is not so common - Voltaire
Visit My GitHub profile to see my open-source projects - https://github.com/paullik
 

Thread Tools




All times are GMT. The time now is 01:39 PM.

VBulletin, Copyright ©2000 - 2014, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.
Copyright 2007 - 2008, www.linux-archive.org