Re: [PATCH 1/4] Add write ring buffer for non-blocking write

[Zhenhua Zhang]

Hi Padovan,

On 02/11/2010 01:23 AM, Gustavo F. Padovan wrote:
* Zhenhua Zhang<zhenhua.zh...@intel.com>  [2010-02-10 16:13:40 +0800]:
+
+       for (i = 0; i<  MAX_BUFFER_NUM; i++) {
+#ifdef WRITE_SCHEDULER_DEBUG
+               buf = ring_buffer_new(4);
+#else
+               buf = ring_buffer_new(4096);
+#endif

Why not a macro to define buf size? So you don't need to use ifdef
everytime

#ifdef WRITE_SCHEDULER_DEBUG
                BUF_SIZE = 4
#else
                BUF_SIZE = 4096 
#endif

Good suggestion. We can use a macro to define the buffer size.

+               if (!buf)
+                       return FALSE;
+
+               server->free_list = g_slist_prepend(server->free_list, buf);
+       }
+
+       return TRUE;
+}
+
+static void free_ring_buffer(struct ring_buffer *buf)
+{
+       ring_buffer_free(buf);
+}

This seems pointless to me.

It just remind me to call ring_buffer_free. Yes. I can remove it and 
call ring_buffer_free in g_slist_foreach() directly.

+
  static void g_at_server_cleanup(GAtServer *server)
  {
        /* Cleanup all received data */
        ring_buffer_free(server->read_buf);
        server->read_buf = NULL;

+       /* Cleanup pending data to write */
+       ring_buffer_free(server->write_buf);
+       server->write_buf = NULL;
+
+       if (server->full_list)
+               g_slist_foreach(server->full_list, (GFunc)free_ring_buffer,
+                                       NULL);
+
+       if (server->free_list)
+               g_slist_foreach(server->free_list, (GFunc)free_ring_buffer,
+                                       NULL);
+
        server->channel = NULL;
  }

@@ -446,8 +507,6 @@ static void read_watcher_destroy_notify(GAtServer *server)
        g_at_server_cleanup(server);
        server->read_watch = 0;

-       server->channel = NULL;
-
        if (server->user_disconnect)
                server->user_disconnect(server->user_disconnect_data);

@@ -455,6 +514,23 @@ static void read_watcher_destroy_notify(GAtServer *server)
                g_free(server);
  }

+static void write_watcher_destroy_notify(GAtServer *server)
+{
+       server->write_watch = 0;
+}
+
+static void g_at_server_wakeup_writer(GAtServer *server)
+{
+       if (server->write_watch != 0)
+               return;
+
+       server->write_watch = g_io_add_watch_full(server->channel,
+                       G_PRIORITY_DEFAULT,
+                       G_IO_OUT | G_IO_HUP | G_IO_ERR | G_IO_NVAL,
+                       can_write_data, server,
+                       (GDestroyNotify)write_watcher_destroy_notify);
+}
+
  static void v250_settings_create(struct v250_settings *v250)
  {
        v250->s3 = '\r';
@@ -483,11 +559,22 @@ GAtServer *g_at_server_new(GIOChannel *io)
        v250_settings_create(&server->v250);
        server->channel = io;
        server->read_buf = ring_buffer_new(4096);
-       server->max_read_attempts = 3;
-
        if (!server->read_buf)
                goto error;

+#ifdef WRITE_SCHEDULER_DEBUG
+       server->write_buf = ring_buffer_new(4);
+#else
+       server->write_buf = ring_buffer_new(4096);
+#endif
+       if (!server->write_buf)
+               goto error;
+
+       if (!alloc_free_list(server))
+               goto error;
+
+       server->max_read_attempts = 3;
+
        if (!g_at_util_setup_io(server->channel, G_IO_FLAG_NONBLOCK))
                goto error;

@@ -502,6 +589,13 @@ error:
        if (server->read_buf)
                ring_buffer_free(server->read_buf);

+       if (server->write_buf)
+               ring_buffer_free(server->write_buf);
+
+       if (server->free_list)
+               g_slist_foreach(server->free_list, (GFunc)free_ring_buffer,
+                                       NULL);
+
        if (server)
                g_free(server);

@@ -552,6 +646,9 @@ gboolean g_at_server_shutdown(GAtServer *server)
        server->user_disconnect = NULL;
        server->user_disconnect_data = NULL;

+       if (server->write_watch)
+               g_source_remove(server->write_watch);
+
        if (server->read_watch)
                g_source_remove(server->read_watch);

--
1.6.6.1

_______________________________________________
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


_______________________________________________
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono