Re: [PATCH xserver 2/4] xfree86: remove unused path from the LoadModule API

2016-04-18 Thread Emil Velikov 
On 18 April 2016 at 22:08, Adam Jackson  wrote:
> On Mon, 2016-04-18 at 21:44 +0100, Emil Velikov wrote:
>> On 18 April 2016 at 21:43, Emil Velikov  wrote:
>> >
>> > On 18 April 2016 at 20:50, Adam Jackson  wrote:
>> > >
>> > > On Sun, 2016-04-17 at 21:07 +0100, Emil Velikov wrote:
>> > > >
>> > > > Similar to its little brothre - LoadSubModule. Currently all call sites
>> > > > provide NULL anyway ;-)
>> > > You can be more aggressive here, subdirlist and patternlist are also
>> > > always NULL. And errmin is almost entirely pointless, I can only find
>> > > one place that it disambiguates.
>> > >
>> > Sure I have patches for that as well. Here is my line of thought
>> >
>> > Step 1 - make LoadModule and LoadSubModule alike
>> > Step 2 - nuke subdirlist, subdirlist, subdirlist and subdirlist, from
>> > both APIs ;-)
>> >
>> Oops the list should be subdirlist, patternlist, options and modreq
>
> I'm not sure options can go? It seems to be the only reasonable way to
> get configuration into, say, the vnc module.
>
Looking at it again... I've misread (and butchered) it. Yes it cannot go :-\

Emil
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xserver 2/4] xfree86: remove unused path from the LoadModule API

2016-04-18 Thread Adam Jackson 
On Mon, 2016-04-18 at 21:44 +0100, Emil Velikov wrote:
> On 18 April 2016 at 21:43, Emil Velikov  wrote:
> > 
> > On 18 April 2016 at 20:50, Adam Jackson  wrote:
> > > 
> > > On Sun, 2016-04-17 at 21:07 +0100, Emil Velikov wrote:
> > > > 
> > > > Similar to its little brothre - LoadSubModule. Currently all call sites
> > > > provide NULL anyway ;-)
> > > You can be more aggressive here, subdirlist and patternlist are also
> > > always NULL. And errmin is almost entirely pointless, I can only find
> > > one place that it disambiguates.
> > > 
> > Sure I have patches for that as well. Here is my line of thought
> > 
> > Step 1 - make LoadModule and LoadSubModule alike
> > Step 2 - nuke subdirlist, subdirlist, subdirlist and subdirlist, from
> > both APIs ;-)
> > 
> Oops the list should be subdirlist, patternlist, options and modreq

I'm not sure options can go? It seems to be the only reasonable way to
get configuration into, say, the vnc module.

- ajax
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xserver 2/4] xfree86: remove unused path from the LoadModule API

2016-04-18 Thread Emil Velikov 
On 18 April 2016 at 21:43, Emil Velikov  wrote:
> On 18 April 2016 at 20:50, Adam Jackson  wrote:
>> On Sun, 2016-04-17 at 21:07 +0100, Emil Velikov wrote:
>>> Similar to its little brothre - LoadSubModule. Currently all call sites
>>> provide NULL anyway ;-)
>>
>> You can be more aggressive here, subdirlist and patternlist are also
>> always NULL. And errmin is almost entirely pointless, I can only find
>> one place that it disambiguates.
>>
> Sure I have patches for that as well. Here is my line of thought
>
> Step 1 - make LoadModule and LoadSubModule alike
> Step 2 - nuke subdirlist, subdirlist, subdirlist and subdirlist, from
> both APIs ;-)
>
Oops the list should be subdirlist, patternlist, options and modreq

-Emil
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xserver 2/4] xfree86: remove unused path from the LoadModule API

2016-04-18 Thread Emil Velikov 
On 18 April 2016 at 20:50, Adam Jackson  wrote:
> On Sun, 2016-04-17 at 21:07 +0100, Emil Velikov wrote:
>> Similar to its little brothre - LoadSubModule. Currently all call sites
>> provide NULL anyway ;-)
>
> You can be more aggressive here, subdirlist and patternlist are also
> always NULL. And errmin is almost entirely pointless, I can only find
> one place that it disambiguates.
>
Sure I have patches for that as well. Here is my line of thought

Step 1 - make LoadModule and LoadSubModule alike
Step 2 - nuke subdirlist, subdirlist, subdirlist and subdirlist, from
both APIs ;-)

-Emil
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xserver 2/4] xfree86: remove unused path from the LoadModule API

2016-04-18 Thread Aaron Plattner 

On 04/17/2016 01:07 PM, Emil Velikov wrote:

Similar to its little brothre - LoadSubModule. Currently all call sites
provide NULL anyway ;-)

Cc: Aaron Plattner 
Signed-off-by: Emil Velikov 
---

Aaron, are you guys using the argument in the closed source driver ?


We don't call LoadModule at all.


-Emil


  hw/xfree86/common/xf86Helper.c  | 2 +-
  hw/xfree86/common/xf86Init.c| 2 +-
  hw/xfree86/doc/ddxDesign.xml| 8 +---
  hw/xfree86/loader/loaderProcs.h | 2 +-
  hw/xfree86/loader/loadmod.c | 7 +++
  5 files changed, 7 insertions(+), 14 deletions(-)

diff --git a/hw/xfree86/common/xf86Helper.c b/hw/xfree86/common/xf86Helper.c
index 3b01a49..2c0f0d8 100644
--- a/hw/xfree86/common/xf86Helper.c
+++ b/hw/xfree86/common/xf86Helper.c
@@ -1626,7 +1626,7 @@ xf86LoadOneModule(const char *name, void *opt)
  return NULL;
  }

-mod = LoadModule(Name, NULL, NULL, NULL, opt, NULL, , );
+mod = LoadModule(Name, NULL, NULL, opt, NULL, , );
  if (!mod)
  LoaderErrorMsg(NULL, Name, errmaj, errmin);
  free(Name);
diff --git a/hw/xfree86/common/xf86Init.c b/hw/xfree86/common/xf86Init.c
index de51497..9b88e29 100644
--- a/hw/xfree86/common/xf86Init.c
+++ b/hw/xfree86/common/xf86Init.c
@@ -1571,7 +1571,7 @@ xf86LoadModules(const char **list, void **optlist)
  else
  opt = NULL;

-if (!LoadModule(name, NULL, NULL, NULL, opt, NULL, , )) {
+if (!LoadModule(name, NULL, NULL, opt, NULL, , )) {
  LoaderErrorMsg(NULL, name, errmaj, errmin);
  failed = TRUE;
  }
diff --git a/hw/xfree86/doc/ddxDesign.xml b/hw/xfree86/doc/ddxDesign.xml
index a8e539c..7baf2cb 100644
--- a/hw/xfree86/doc/ddxDesign.xml
+++ b/hw/xfree86/doc/ddxDesign.xml
@@ -5222,7 +5222,7 @@ XFree86 common layer.


  
-pointer LoadModule(const char *module, const char *path,
+pointer LoadModule(const char *module,
 const char **subdirlist, const char **patternlist,
 pointer options, const XF86ModReqInfo * modreq,
 int *errmaj, int *errmin);
@@ -5238,12 +5238,6 @@ XFree86 common layer.
  This might change.  The other parameters are:

  
-   
- path
- 
- An optional comma-separated list of module search paths.
- When NULL, the default search path is 
used.
-   



diff --git a/hw/xfree86/loader/loaderProcs.h b/hw/xfree86/loader/loaderProcs.h
index cfc4d80..8d7872f 100644
--- a/hw/xfree86/loader/loaderProcs.h
+++ b/hw/xfree86/loader/loaderProcs.h
@@ -74,7 +74,7 @@ void LoaderInit(void);

  ModuleDescPtr LoadDriver(const char *, const char *, int, void *, int *,
   int *);
-ModuleDescPtr LoadModule(const char *, const char *, const char **,
+ModuleDescPtr LoadModule(const char *, const char **,
   const char **, void *, const XF86ModReqInfo *,
   int *, int *);
  ModuleDescPtr DuplicateModule(ModuleDescPtr mod, ModuleDescPtr parent);
diff --git a/hw/xfree86/loader/loadmod.c b/hw/xfree86/loader/loadmod.c
index 702d4e7..603ef65 100644
--- a/hw/xfree86/loader/loadmod.c
+++ b/hw/xfree86/loader/loadmod.c
@@ -747,7 +747,7 @@ LoadSubModule(void *_parent, const char *module,
  return NULL;
  }

-submod = LoadModule(module, NULL, subdirlist, patternlist, options,
+submod = LoadModule(module, subdirlist, patternlist, options,
  modreq, errmaj, errmin);
  if (submod && submod != (ModuleDescPtr) 1) {
  parent->child = AddSibling(parent->child, submod);
@@ -817,7 +817,6 @@ static const char *compiled_in_modules[] = {
   * module   The module name.  Normally this is not a filename but the
   *  module's "canonical name.  A full pathname is, however,
   *  also accepted.
- * path A comma separated list of module directories.
   * subdirlist   A NULL terminated list of subdirectories to search.  When
   *  NULL, the default "stdSubdirs" list is used.  The default
   *  list is also substituted for entries with value DEFAULT_LIST.
@@ -849,7 +848,7 @@ static const char *compiled_in_modules[] = {
   *
   */
  ModuleDescPtr
-LoadModule(const char *module, const char *path, const char **subdirlist,
+LoadModule(const char *module, const char **subdirlist,
 const char **patternlist, void *options,
 const XF86ModReqInfo * modreq, int *errmaj, int *errmin)
  {
@@ -905,7 +904,7 @@ LoadModule(const char *module, const char *path, const char 
**subdirlist,
  goto LoadModule_fail;
  }

-pathlist = InitPathList(path);
+pathlist = InitPathList(NULL);
  if (!pathlist) {
  /* This could be a malloc failure too */
  if (errmaj)



Reviewed-by: Aaron Plattner

[PATCH xserver] dix: Remove pointless client-state callbacks

2016-04-18 Thread Adam Jackson 
Private storage is pre-zeroed by the private system itself.

Signed-off-by: Adam Jackson 
---
 Xext/geext.c  | 22 --
 Xi/extinit.c  | 15 ---
 composite/compext.c   | 14 --
 damageext/damageext.c | 23 +--
 render/render.c   | 13 -
 xfixes/xfixes.c   | 21 +
 6 files changed, 2 insertions(+), 106 deletions(-)

diff --git a/Xext/geext.c b/Xext/geext.c
index 6285f69..6312f24 100644
--- a/Xext/geext.c
+++ b/Xext/geext.c
@@ -145,28 +145,10 @@ SProcGEDispatch(ClientPtr client)
 return (*SProcGEVector[stuff->ReqType]) (client);
 }
 
-/**
- * Called when a new client inits a connection to the X server.
- *
- * We alloc a simple struct to store the client's major/minor version. Can be
- * used in the furture for versioning support.
- */
-static void
-GEClientCallback(CallbackListPtr *list, void *closure, void *data)
-{
-NewClientInfoRec *clientinfo = (NewClientInfoRec *) data;
-ClientPtr pClient = clientinfo->client;
-GEClientInfoPtr pGEClient = GEGetClient(pClient);
-
-pGEClient->major_version = 0;
-pGEClient->minor_version = 0;
-}
-
 /* Reset extension. Called on server shutdown. */
 static void
 GEResetProc(ExtensionEntry * extEntry)
 {
-DeleteCallback(, GEClientCallback, 0);
 EventSwapVector[GenericEvent] = NotImplemented;
 }
 
@@ -205,10 +187,6 @@ GEExtensionInit(void)
 (, PRIVATE_CLIENT, sizeof(GEClientInfoRec)))
 FatalError("GEExtensionInit: GE private request failed.\n");
 
-if (!AddCallback(, GEClientCallback, 0)) {
-FatalError("GEExtensionInit: register client callback failed.\n");
-}
-
 if ((extEntry = AddExtension(GE_NAME,
  0, GENumberErrors,
  ProcGEDispatch, SProcGEDispatch,
diff --git a/Xi/extinit.c b/Xi/extinit.c
index 26c628c..48edf38 100644
--- a/Xi/extinit.c
+++ b/Xi/extinit.c
@@ -380,18 +380,6 @@ DevPrivateKeyRec XIClientPrivateKeyRec;
  *
  */
 
-static void
-XIClientCallback(CallbackListPtr *list, void *closure, void *data)
-{
-NewClientInfoRec *clientinfo = (NewClientInfoRec *) data;
-ClientPtr pClient = clientinfo->client;
-XIClientPtr pXIClient;
-
-pXIClient = dixLookupPrivate(>devPrivates, XIClientPrivateKey);
-pXIClient->major_version = 0;
-pXIClient->minor_version = 0;
-}
-
 /*
  *
  * ProcIDispatch - main dispatch routine for requests to this extension.
@@ -1296,9 +1284,6 @@ XInputExtensionInit(void)
 (, PRIVATE_CLIENT, sizeof(XIClientRec)))
 FatalError("Cannot request private for XI.\n");
 
-if (!AddCallback(, XIClientCallback, 0))
-FatalError("Failed to add callback to XI.\n");
-
 if (!XIBarrierInit())
 FatalError("Could not initialize barriers.\n");
 
diff --git a/composite/compext.c b/composite/compext.c
index f1a8255..b95bf99 100644
--- a/composite/compext.c
+++ b/composite/compext.c
@@ -66,17 +66,6 @@ typedef struct _CompositeClient {
 #define GetCompositeClient(pClient) ((CompositeClientPtr) \
 dixLookupPrivate(&(pClient)->devPrivates, CompositeClientPrivateKey))
 
-static void
-CompositeClientCallback(CallbackListPtr *list, void *closure, void *data)
-{
-NewClientInfoRec *clientinfo = (NewClientInfoRec *) data;
-ClientPtr pClient = clientinfo->client;
-CompositeClientPtr pCompositeClient = GetCompositeClient(pClient);
-
-pCompositeClient->major_version = 0;
-pCompositeClient->minor_version = 0;
-}
-
 static int
 FreeCompositeClientWindow(void *value, XID ccwid)
 {
@@ -580,9 +569,6 @@ CompositeExtensionInit(void)
sizeof(CompositeClientRec)))
 return;
 
-if (!AddCallback(, CompositeClientCallback, 0))
-return;
-
 for (s = 0; s < screenInfo.numScreens; s++)
 if (!compScreenInit(screenInfo.screens[s]))
 return;
diff --git a/damageext/damageext.c b/damageext/damageext.c
index 886f56d..86b54ee 100644
--- a/damageext/damageext.c
+++ b/damageext/damageext.c
@@ -568,24 +568,6 @@ SProcDamageDispatch(ClientPtr client)
 return (*SProcDamageVector[stuff->damageReqType]) (client);
 }
 
-static void
-DamageClientCallback(CallbackListPtr *list, void *closure, void *data)
-{
-NewClientInfoRec *clientinfo = (NewClientInfoRec *) data;
-ClientPtr pClient = clientinfo->client;
-DamageClientPtr pDamageClient = GetDamageClient(pClient);
-
-pDamageClient->critical = 0;
-pDamageClient->major_version = 0;
-pDamageClient->minor_version = 0;
-}
-
- /*ARGSUSED*/ static void
-DamageResetProc(ExtensionEntry * extEntry)
-{
-DeleteCallback(, DamageClientCallback, 0);
-}
-
 static int
 FreeDamageExt(void *value, XID did)
 {
@@ -757,13 +739,10 @@ DamageExtensionInit(void)
 (, PRIVATE_CLIENT, sizeof(DamageClientRec)))
 return;
 
-if (!AddCallback(,

Re: [PATCH libICE] Enable visibility annotations

2016-04-18 Thread Alan Coopersmith 

_IceTransNoListen


While that's absolutely not part of the public API, and no one should have
ever called it, since we utterly failed to give them any way to provide a
reasonable security mode without it, I believe there are several important
callers in the various DE's.

--
-Alan Coopersmith-  alan.coopersm...@oracle.com
 Oracle Solaris Engineering - http://blogs.oracle.com/alanc
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xserver 4/4] xfree86/parser: simplify #ifdef ladder

2016-04-18 Thread Adam Jackson 
On Mon, 2016-04-18 at 11:01 -0400, Alex Deucher wrote:
> On Sun, Apr 17, 2016 at 3:30 PM, Emil Velikov  wrote:
> > 
> > Rather than 'hacking' around symbol names and providing macros such as
> > 'Local' just fold things and make the code more readable.
> > 
> > Signed-off-by: Emil Velikov 
> Series is:
> Reviewed-by: Alex Deucher 

remote: I: patch #81544 updated using rev 
b93be14b7d339e4e46d941729dad853452fae8c0.
remote: I: patch #81545 updated using rev 
944ea03d5be2ffe22a3f1c4c287760261c31235f.
remote: I: patch #81546 updated using rev 
537276a5b86b7341169ea4eb36d479a503ba5d84.
remote: I: patch #81547 updated using rev 
577bebe2067293bb154068e99a2ef085b066cb67.
remote: I: 4 patch(es) updated to state Accepted.
To ssh://git.freedesktop.org/git/xorg/xserver
   3981dcd..577bebe  master -> master

- ajax
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xserver] dri3: remove unused file dri3int.h

2016-04-18 Thread Adam Jackson 
On Mon, 2016-04-18 at 10:58 -0400, Alex Deucher wrote:
> On Sun, Apr 17, 2016 at 3:34 PM, Emil Velikov  wrote:
> > 
> > Copied during the prototyping stage and never used.
> > 
> > Cc: Keith Packard 
> > Signed-off-by: Emil Velikov 
> Reviewed-by: Alex Deucher 

remote: I: patch #81548 updated using rev 
3981dcdd489b60fbf356534a509ca93dcbedf769.
remote: I: 1 patch(es) updated to state Accepted.
To ssh://git.freedesktop.org/git/xorg/xserver
   a1b13cd..3981dcd  master -> master

- ajax
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH libICE] Enable visibility annotations

2016-04-18 Thread Adam Jackson 
On Mon, 2016-04-18 at 09:23 +0300, Yury Gribov wrote:

> Does below look like a sane approach?
> 1) get all deps via
>    $ apt-cache rdepends libice6 libice-dev
> 2) unpack each of the above .debs and scan for ELFs
> 3) for each ELF see if one of now hidden symbols is UND

That sounds good to me.

> Note that this does not handle transitive dependencies e.g. if some 
> weird library links static version of libICE and the re-exports it's 
> symbols as part of new lib's public interface.

It'd be possible to scan for this too I suspect.  Look for packages
that BuildRequire libice6-static, scan the resulting binaries for
exports. I suspect there are zero such packages.

- ajax
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xserver] xwayland: Fix compiler warning in GLAMOR Xv

2016-04-18 Thread Adam Jackson 
On Sat, 2016-04-16 at 01:52 +0100, Emil Velikov wrote:

> Is there a specific reason behind the (seemingly) duplicate Xv/XvMC
> code in xserver - {Encoding, Format, Image...}Rec. Seems to me that
> the top Xext should be the sole one, although xf86, xwin, xwayland and
> kdrive have their (almost) identical versions.
> 
> I'd imagine it's a case of people lacking the time to folding things
> up or is that an intentional split ?

More the former. Many of the di/dd splits in the code seem to me like
they start from very conservative assumptions about what "the hardware"
might be capable of, and hardware just doesn't vary that much.

If someone did want to drain that particular swamp, it'd be nice to see
Xv support in Xnest first, to ensure that the ensuing refactor is still
expressive enough to handle servers without pixmaps.

- ajax
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xserver 4/4] xfree86/parser: simplify #ifdef ladder

2016-04-18 Thread Alex Deucher 
On Sun, Apr 17, 2016 at 3:30 PM, Emil Velikov  wrote:
> Rather than 'hacking' around symbol names and providing macros such as
> 'Local' just fold things and make the code more readable.
>
> Signed-off-by: Emil Velikov 

Series is:
Reviewed-by: Alex Deucher 

> ---
>
> If people prefer we can split out the different implementations
> (HAS_SAVED_IDS_AND_SETEUID vs not) to separate functions and call if
> from xf86writeConfigFile(). I don't mind either way.
>
> -Emil
>
>  hw/xfree86/parser/write.c | 31 +--
>  1 file changed, 9 insertions(+), 22 deletions(-)
>
> diff --git a/hw/xfree86/parser/write.c b/hw/xfree86/parser/write.c
> index 8792783..9a24dd6 100644
> --- a/hw/xfree86/parser/write.c
> +++ b/hw/xfree86/parser/write.c
> @@ -73,14 +73,7 @@
>  #define HAS_NO_UIDS
>  #endif
>
> -#ifdef HAS_NO_UIDS
> -#define doWriteConfigFile xf86writeConfigFile
> -#define Local /**/
> -#else
> -#define Local static
> -#endif
> -
> -Local int
> +static int
>  doWriteConfigFile(const char *filename, XF86ConfigPtr cptr)
>  {
>  FILE *cf;
> @@ -134,24 +127,19 @@ doWriteConfigFile(const char *filename, XF86ConfigPtr 
> cptr)
>  return 1;
>  }
>
> -#ifndef HAS_NO_UIDS
> -
>  int
>  xf86writeConfigFile(const char *filename, XF86ConfigPtr cptr)
>  {
> +#ifndef HAS_NO_UIDS
>  int ret;
>
> -#if !defined(HAS_SAVED_IDS_AND_SETEUID)
> -int pid, p;
> -int status;
> -void (*csig) (int);
> -#else
> -int ruid, euid;
> -#endif
> -
>  if (getuid() != geteuid()) {
>
>  #if !defined(HAS_SAVED_IDS_AND_SETEUID)
> +int pid, p;
> +int status;
> +void (*csig) (int);
> +
>  /* Need to fork to change ruid without loosing euid */
>  csig = signal(SIGCHLD, SIG_DFL);
>  switch ((pid = fork())) {
> @@ -178,6 +166,7 @@ xf86writeConfigFile(const char *filename, XF86ConfigPtr 
> cptr)
>  return 0;
>
>  #else   /* HAS_SAVED_IDS_AND_SETEUID */
> +int ruid, euid;
>
>  ruid = getuid();
>  euid = geteuid();
> @@ -198,9 +187,7 @@ xf86writeConfigFile(const char *filename, XF86ConfigPtr 
> cptr)
>  #endif  /* HAS_SAVED_IDS_AND_SETEUID */
>
>  }
> -else {
> +else
> +#endif  /* !HAS_NO_UIDS */
>  return doWriteConfigFile(filename, cptr);
> -}
>  }
> -
> -#endif  /* !HAS_NO_UIDS */
> --
> 2.8.0
>
> ___
> xorg-devel@lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: https://lists.x.org/mailman/listinfo/xorg-devel
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xserver] dri3: remove unused file dri3int.h

2016-04-18 Thread Alex Deucher 
On Sun, Apr 17, 2016 at 3:34 PM, Emil Velikov  wrote:
> Copied during the prototyping stage and never used.
>
> Cc: Keith Packard 
> Signed-off-by: Emil Velikov 

Reviewed-by: Alex Deucher 

> ---
>  dri3/dri3int.h | 26 --
>  1 file changed, 26 deletions(-)
>  delete mode 100644 dri3/dri3int.h
>
> diff --git a/dri3/dri3int.h b/dri3/dri3int.h
> deleted file mode 100644
> index 7f53eba..000
> --- a/dri3/dri3int.h
> +++ /dev/null
> @@ -1,26 +0,0 @@
> -/*
> - * Copyright © 2011 Daniel Stone
> - *
> - * Permission is hereby granted, free of charge, to any person obtaining a
> - * copy of this software and associated documentation files (the "Software"),
> - * to deal in the Software without restriction, including without limitation
> - * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> - * and/or sell copies of the Software, and to permit persons to whom the
> - * Software is furnished to do so, subject to the following conditions:
> - *
> - * The above copyright notice and this permission notice (including the next
> - * paragraph) shall be included in all copies or substantial portions of the
> - * Software.
> - *
> - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> - * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> - * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> - * DEALINGS IN THE SOFTWARE.
> - *
> - * Author: Daniel Stone 
> - */
> -
> -extern Bool DRI2ModuleSetup(void);
> --
> 2.8.0
>
> ___
> xorg-devel@lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: https://lists.x.org/mailman/listinfo/xorg-devel
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH libICE] Enable visibility annotations

2016-04-18 Thread Yury Gribov 

On 04/15/2016 07:40 PM, Adam Jackson wrote:

On Thu, 2016-04-14 at 09:50 +0300, Yury Gribov wrote:

Hi all,

This patch adds visibility annotations to public functions in libICE and
also enables -fvisibility=hidden on platforms where it's available.

This results in 10% size decrease (101K -> 92K) and 50% export symbols
reduction (141 -> 67) on x86_64.


I'm not sure how you're getting those numbers? 'nm -D --defined' is
giving me 138 exported before and 100 after, and:

text   data bss dec hex filename
   90853   3000   14560  108413   1a77d libICE.so.before
   87507   2712   14560  104779   1994b libICE.so.after

More like 4k (3.3%) size decrease.


Hm, I originally used readelf but it returns (more or less) same results 
as nm:


$ nm -D --defined ~/install/libICE/usr/local/lib/libICE.so | wc -l
138
$ nm -D --defined ~/install/libICE-patched/usr/local/lib/libICE.so | wc -l
64

$ readelf -sDW ~/install/libICE/usr/local/lib/libICE.so | grep -v UND | 
wc -l

141
$ readelf -sDW ~/install/libICE-patched/usr/local/lib/libICE.so | grep 
-v UND | wc -l

67

It seems that some of the hidden symbols depend on system X11 headers 
e.g. _IceTransAccept from icetrans.c uses 
/usr/include/X11/Xtrans/transport.c which may be different on your system.


I've attached a list of old vs. new symbols on my system for reference.


I'm a little cautious about patches like this. The X libraries have had
an effectively static ABI for ages, so even functions that aren't
exposed in the installed headers might find themselves in use. I'd like
to at least know that, say, no app or lib in the Debian archive is
importing any of the newly hidden symbols. Have you checked?


I totally agree that patches like this should be checked before deploy 
(even though using functions which are not a part of librarypublic 
interface may be considered a bad practice).


Does below look like a sane approach?
1) get all deps via
  $ apt-cache rdepends libice6 libice-dev
2) unpack each of the above .debs and scan for ELFs
3) for each ELF see if one of now hidden symbols is UND

Note that this does not handle transitive dependencies e.g. if some 
weird library links static version of libICE and the re-exports it's 
symbols as part of new lib's public interface.


-Y
__bss_start
_edata
_end
_fini
IceAcceptConnection
IceAddConnectionWatch
IceAllocScratch
IceAppLockConn
IceAppUnlockConn
IceAuthFileName
IceCheckShutdownNegotiation
IceCloseConnection
IceComposeNetworkIdList
IceConnectionNumber
IceConnectionStatus
IceConnectionString
_IceErrorBadLength
_IceErrorBadMinor
_IceErrorBadState
_IceErrorBadValue
IceFlush
IceFreeAuthFileEntry
IceFreeListenObjs
IceGenerateMagicCookie
IceGetAuthFileEntry
IceGetConnectionContext
IceGetInBufSize
IceGetListenConnectionNumber
IceGetListenConnectionString
IceGetOutBufSize
IceGetPeerName
IceInitThreads
IceLastReceivedSequenceNumber
IceLastSentSequenceNumber
IceListenForConnections
IceListenForWellKnownConnections
IceLockAuthFile
IceOpenConnection
_IcePaMagicCookie1Proc
IcePing
_IcePoMagicCookie1Proc
IceProcessMessages
IceProtocolRevision
IceProtocolSetup
IceProtocolShutdown
IceProtocolVersion
_IceRead
IceReadAuthFileEntry
_IceReadSkip
IceRegisterForProtocolReply
IceRegisterForProtocolSetup
IceRelease
IceRemoveConnectionWatch
IceSetErrorHandler
IceSetHostBasedAuthProc
IceSetIOErrorHandler
IceSetPaAuthData
IceSetShutdownNegotiation
IceSwapping
IceUnlockAuthFile
IceVendor
_IceWrite
IceWriteAuthFileEntry
_init
__bss_start
_edata
_end
_fini
IceAcceptConnection
IceAddConnectionWatch
_IceAddOpcodeMapping
_IceAddReplyWait
IceAllocScratch
IceAppLockConn
IceAppUnlockConn
_IceAuthCount
IceAuthFileName
_IceAuthNames
_IceCheckReplyReady
IceCheckShutdownNegotiation
IceCloseConnection
IceComposeNetworkIdList
_IceConnectionClosed
_IceConnectionCount
IceConnectionNumber
_IceConnectionObjs
_IceConnectionOpened
IceConnectionStatus
IceConnectionString
_IceConnectionStrings
_IceErrorAuthenticationFailed
_IceErrorAuthenticationRejected
_IceErrorBadLength
_IceErrorBadMajor
_IceErrorBadMinor
_IceErrorBadState
_IceErrorBadValue
_IceErrorHandler
_IceErrorMajorOpcodeDuplicate
_IceErrorNoAuthentication
_IceErrorNoVersion
_IceErrorProtocolDuplicate
_IceErrorSetupFailed
_IceErrorUnknownProtocol
IceFlush
IceFreeAuthFileEntry
_IceFreeConnection
IceFreeListenObjs
IceGenerateMagicCookie
IceGetAuthFileEntry
IceGetConnectionContext
IceGetInBufSize
IceGetListenConnectionNumber
IceGetListenConnectionString
IceGetOutBufSize
_IceGetPaAuthData
_IceGetPaValidAuthIndices
IceGetPeerName
_IceGetPeerName
_IceGetPoAuthData
_IceGetPoValidAuthIndices
IceInitThreads
_IceIOErrorHandler
_IceLastMajorOpcode
IceLastReceivedSequenceNumber
IceLastSentSequenceNumber
IceListenForConnections
IceListenForWellKnownConnections
IceLockAuthFile
IceOpenConnection
_IcePaAuthDataEntries
_IcePaAuthDataEntryCount
_IcePaAuthProcs
_IcePaMagicCookie1Proc
IcePing
_IcePoAuthProcs
_IcePoMagicCookie1Proc
IceProcessMessages
IceProtocolRevision
_IceProtocols