[directfb-dev] Re: [PATCH] Add IDirectFBDisplayLayer::GetName

[Ville Syrjälä <syrjala@sci.fi>]

On Wed, Dec 04, 2002 at 01:41:13PM +0100, Sven Neumann wrote:
> Hi,
> 
> Ville Syrjälä <syrjala@sci.fi> writes:
> 
> > It appears that a layer's name isn't used anywhere. Or maybe I'm blind?
> > I was going to use it to identify the Matrox G400 layers since hardcoded
> > IDs arent very nice. This patch adds IDirectFBDisplayLayer::GetName.
> > Comments?
> 
> why don't we make it part of the LayerDescription?

Surely would fit the description :) I don't have any preference to either
way.

> The real problem with names and other string values is that you either
> need to copy the string or you expose internal data. If we copy the
> name, we can't make it part of the LayerDescription since that would
> mean that people would have to free it afterwards. We'd have to go for
> IDirectFBDisplayLayer::GetName() then.

True.

> Your patch uses the GetName() API but it doesn't copy the
> string. Since it does not even mark it as const, people may try to
> free it. Since that would lead to some hard-to-debug problems later,
> I'd say we should return a copy instead.

You're right. Copying seems better to avoid segfaults and whatnot. It's
just up to the user to free it. I modified the patch to use strdup() and
added a comment about freeing the memory.

-- 
Ville Syrjälä
syrjala@sci.fi
http://www.sci.fi/~syrjala/
Index: include/directfb.h
===================================================================
RCS file: /cvs/directfb/DirectFB/include/directfb.h,v
retrieving revision 1.158
diff -u -r1.158 directfb.h
--- include/directfb.h	21 Nov 2002 15:50:53 -0000	1.158
+++ include/directfb.h	4 Dec 2002 00:43:54 -0000
@@ -1393,6 +1393,14 @@
      );
 
      /*
+      * Get a copy of the layer name. Caller should free it after use.
+      */
+     DFBResult (*GetName) (
+          IDirectFBDisplayLayer              *thiz,
+          char                              **layer_name
+     );
+
+     /*
       * Get a description of this display layer, i.e. the capabilities.
       */
      DFBResult (*GetDescription) (
Index: src/core/layers.c
===================================================================
RCS file: /cvs/directfb/DirectFB/src/core/layers.c,v
retrieving revision 1.85
diff -u -r1.85 layers.c
--- src/core/layers.c	29 Nov 2002 20:09:02 -0000	1.85
+++ src/core/layers.c	4 Dec 2002 00:44:02 -0000
@@ -923,6 +923,13 @@
      *desc = layer->shared->layer_info.desc;
 }
 
+void
+dfb_layer_name( const DisplayLayer  *layer,
+                char               **name )
+{
+     *name = layer->shared->layer_info.name;
+}
+
 DFBDisplayLayerID
 dfb_layer_id( const DisplayLayer *layer )
 {
Index: src/core/layers.h
===================================================================
RCS file: /cvs/directfb/DirectFB/src/core/layers.h,v
retrieving revision 1.41
diff -u -r1.41 layers.h
--- src/core/layers.h	22 Nov 2002 21:18:00 -0000	1.41
+++ src/core/layers.h	4 Dec 2002 00:44:02 -0000
@@ -283,6 +283,8 @@
 CardState         *dfb_layer_state( DisplayLayer *layer );
 void               dfb_layer_description( const DisplayLayer         *layer,
                                           DFBDisplayLayerDescription *desc );
+void               dfb_layer_name( const DisplayLayer  *layer,
+                                   char               **name );
 DFBDisplayLayerID  dfb_layer_id( const DisplayLayer *layer );
 
 DFBResult          dfb_layer_flip_buffers ( DisplayLayer        *layer,
Index: src/display/idirectfbdisplaylayer.c
===================================================================
RCS file: /cvs/directfb/DirectFB/src/display/idirectfbdisplaylayer.c,v
retrieving revision 1.43
diff -u -r1.43 idirectfbdisplaylayer.c
--- src/display/idirectfbdisplaylayer.c	18 Nov 2002 18:42:43 -0000	1.43
+++ src/display/idirectfbdisplaylayer.c	4 Dec 2002 00:44:15 -0000
@@ -111,6 +111,24 @@
 }
 
 static DFBResult
+IDirectFBDisplayLayer_GetName( IDirectFBDisplayLayer  *thiz,
+                               char                  **name )
+{
+     char *layer_name;
+
+     INTERFACE_GET_DATA(IDirectFBDisplayLayer)
+
+     if (!name)
+          return DFB_INVARG;
+
+     dfb_layer_name( data->layer, &layer_name );
+
+     *name = strdup( layer_name );
+
+     return DFB_OK;
+}
+
+static DFBResult
 IDirectFBDisplayLayer_GetDescription( IDirectFBDisplayLayer      *thiz,
                                       DFBDisplayLayerDescription *desc )
 {
@@ -586,6 +604,7 @@
      thiz->AddRef = IDirectFBDisplayLayer_AddRef;
      thiz->Release = IDirectFBDisplayLayer_Release;
      thiz->GetID = IDirectFBDisplayLayer_GetID;
+     thiz->GetName = IDirectFBDisplayLayer_GetName;
      thiz->GetDescription = IDirectFBDisplayLayer_GetDescription;
      thiz->GetSurface = IDirectFBDisplayLayer_GetSurface;
      thiz->SetCooperativeLevel = IDirectFBDisplayLayer_SetCooperativeLevel;