6:50 a.m.
This removes the "free a string object but return the internal data"
functionality of l_string_free. The option to not free everything was a
source of confusion.
l_string_unwrap is the same as the old l_string_free(ptr, false).
---
ell/dbus-filter.c | 4 ++--
ell/dbus-service.c | 2 +-
ell/dbus-util.c | 4 ++--
ell/gvariant-util.c | 4 ++--
ell/key.c | 2 +-
ell/settings.c | 2 +-
ell/string.c | 32 +++++++++++++++++++++-----------
ell/string.h | 3 ++-
8 files changed, 32 insertions(+), 21 deletions(-)
diff --git a/ell/dbus-filter.c b/ell/dbus-filter.c
index 9e3cad5..0af1b67 100644
--- a/ell/dbus-filter.c
+++ b/ell/dbus-filter.c
@@ -393,7 +393,7 @@ char *_dbus_filter_rule_to_str(const struct _dbus_filter_condition
*rule,
rule->type - L_DBUS_MATCH_ARG0);
break;
default:
- l_string_free(str, true);
+ l_string_free(str);
return NULL;
}
@@ -417,5 +417,5 @@ char *_dbus_filter_rule_to_str(const struct _dbus_filter_condition
*rule,
l_string_append_c(str, ',');
}
- return l_string_free(str, false);
+ return l_string_unwrap(str);
}
diff --git a/ell/dbus-service.c b/ell/dbus-service.c
index 98f6e7a..8bc9244 100644
--- a/ell/dbus-service.c
+++ b/ell/dbus-service.c
@@ -1600,7 +1600,7 @@ bool _dbus_object_tree_dispatch(struct _dbus_object_tree *tree,
buf = l_string_new(0);
_dbus_object_tree_introspect(tree, path, buf);
- xml = l_string_free(buf, false);
+ xml = l_string_unwrap(buf);
reply = l_dbus_message_new_method_return(message);
l_dbus_message_set_arguments(reply, "s", xml);
diff --git a/ell/dbus-util.c b/ell/dbus-util.c
index 1298228..c4e8419 100644
--- a/ell/dbus-util.c
+++ b/ell/dbus-util.c
@@ -901,7 +901,7 @@ void _dbus1_builder_free(struct dbus_builder *builder)
if (unlikely(!builder))
return;
- l_string_free(builder->signature, true);
+ l_string_free(builder->signature);
l_queue_destroy(builder->containers,
(l_queue_destroy_func_t) container_free);
l_free(builder->body);
@@ -1265,7 +1265,7 @@ char *_dbus1_builder_finish(struct dbus_builder *builder,
if (unlikely(l_queue_length(builder->containers) != 1))
return NULL;
- signature = l_string_free(builder->signature, false);
+ signature = l_string_unwrap(builder->signature);
builder->signature = NULL;
*body = builder->body;
diff --git a/ell/gvariant-util.c b/ell/gvariant-util.c
index 2172ad9..e6f70ad 100644
--- a/ell/gvariant-util.c
+++ b/ell/gvariant-util.c
@@ -918,7 +918,7 @@ void _gvariant_builder_free(struct dbus_builder *builder)
if (unlikely(!builder))
return;
- l_string_free(builder->signature, true);
+ l_string_free(builder->signature);
l_queue_destroy(builder->containers,
(l_queue_destroy_func_t) container_free);
l_free(builder->body);
@@ -1319,7 +1319,7 @@ char *_gvariant_builder_finish(struct dbus_builder *builder,
root = l_queue_peek_head(builder->containers);
- signature = l_string_free(builder->signature, false);
+ signature = l_string_unwrap(builder->signature);
builder->signature = NULL;
if (_gvariant_is_fixed_size(signature)) {
diff --git a/ell/key.c b/ell/key.c
index 370b3c8..11700e8 100644
--- a/ell/key.c
+++ b/ell/key.c
@@ -169,7 +169,7 @@ static char *format_key_info(const char *encoding, const char *hash)
if (hash)
l_string_append_printf(info, "hash=%s", hash);
- return l_string_free(info, false);
+ return l_string_unwrap(info);
}
static long kernel_query_key(int32_t key_serial, const char *encoding,
diff --git a/ell/settings.c b/ell/settings.c
index c6eec35..08ba967 100644
--- a/ell/settings.c
+++ b/ell/settings.c
@@ -427,7 +427,7 @@ LIB_EXPORT char *l_settings_to_data(struct l_settings *settings,
size_t *len)
group_entry = group_entry->next;
}
- ret = l_string_free(buf, false);
+ ret = l_string_unwrap(buf);
if (len)
*len = strlen(ret);
diff --git a/ell/string.c b/ell/string.c
index 98e1c46..e6e7140 100644
--- a/ell/string.c
+++ b/ell/string.c
@@ -117,27 +117,37 @@ LIB_EXPORT struct l_string *l_string_new(size_t initial_length)
/**
* l_string_free:
* @string: growable string object
+ *
+ * Free the growable string object and all associated data
+ **/
+LIB_EXPORT void l_string_free(struct l_string *string)
+{
+ if (unlikely(!string))
+ return;
+
+ l_free(string->str);
+ l_free(string);
+}
+
+/**
+ * l_string_unwrap:
+ * @string: growable string object
* @free_data: internal string array
*
- * Free the growable string object. If @free_data #true, then the internal
- * string data will be freed and NULL will be returned. Otherwise the
- * internal string data will be returned to the caller. The caller is
- * responsible for freeing it using l_free().
+ * Free the growable string object and return the internal string data.
+ * The caller is responsible for freeing the string data using l_free(),
+ * and the string object is no longer usable.
*
- * Returns: @string's internal buffer or NULL
+ * Returns: @string's internal buffer
**/
-LIB_EXPORT char *l_string_free(struct l_string *string, bool free_data)
+LIB_EXPORT char *l_string_unwrap(struct l_string *string)
{
char *result;
if (unlikely(!string))
return NULL;
- if (free_data) {
- l_free(string->str);
- result = NULL;
- } else
- result = string->str;
+ result = string->str;
l_free(string);
diff --git a/ell/string.h b/ell/string.h
index a778b31..6549a05 100644
--- a/ell/string.h
+++ b/ell/string.h
@@ -100,7 +100,8 @@ static inline bool __attribute__ ((always_inline))
}
struct l_string *l_string_new(size_t initial_length);
-char *l_string_free(struct l_string *string, bool free_data);
+void l_string_free(struct l_string *string);
+char *l_string_unwrap(struct l_string *string);
struct l_string *l_string_append(struct l_string *dest, const char *src);
struct l_string *l_string_append_c(struct l_string *dest, const char c);
--
2.10.2
6:50 a.m.
New subject: [PATCH 2/5] unit: Update for l_string_free changes
---
unit/test-dbus-service.c | 10 +++++-----
unit/test-string.c | 10 +++++-----
2 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/unit/test-dbus-service.c b/unit/test-dbus-service.c
index c27869b..269e0e6 100644
--- a/unit/test-dbus-service.c
+++ b/unit/test-dbus-service.c
@@ -116,7 +116,7 @@ static void test_introspect_method(const void *test_data)
buf = l_string_new(0);
_dbus_method_introspection(method, buf);
- xml = l_string_free(buf, false);
+ xml = l_string_unwrap(buf);
assert(!strcmp(test->expected_xml, xml));
l_free(xml);
@@ -134,7 +134,7 @@ static void test_introspect_signal(const void *test_data)
buf = l_string_new(0);
_dbus_signal_introspection(signal, buf);
- xml = l_string_free(buf, false);
+ xml = l_string_unwrap(buf);
assert(!strcmp(test->expected_xml, xml));
l_free(xml);
@@ -152,7 +152,7 @@ static void test_introspect_property(const void *test_data)
buf = l_string_new(0);
_dbus_property_introspection(property, buf);
- xml = l_string_free(buf, false);
+ xml = l_string_unwrap(buf);
assert(!strcmp(test->expected_xml, xml));
l_free(xml);
@@ -166,7 +166,7 @@ static void test_introspect_interface(const void *test_data)
buf = l_string_new(0);
_dbus_interface_introspection(interface, buf);
- xml = l_string_free(buf, false);
+ xml = l_string_unwrap(buf);
assert(!strcmp(test->expected_xml, xml));
l_free(xml);
@@ -358,7 +358,7 @@ static void test_dbus_object_tree_introspection(const void
*test_data)
buf = l_string_new(1024);
_dbus_object_tree_introspect(tree, "/", buf);
- xml = l_string_free(buf, false);
+ xml = l_string_unwrap(buf);
assert(!strcmp(ofono_manager_introspection, xml));
l_free(xml);
diff --git a/unit/test-string.c b/unit/test-string.c
index 4c6a38c..d5b0d9d 100644
--- a/unit/test-string.c
+++ b/unit/test-string.c
@@ -40,7 +40,7 @@ static void test_grow(const void *test_data)
assert(l_string_append(str, "BarFoo"));
assert(l_string_length(str) == strlen("Foobar7BarFoo"));
- a = l_string_free(str, false);
+ a = l_string_unwrap(str);
assert(a);
assert(!strcmp(a, "Foobar7BarFoo"));
@@ -60,7 +60,7 @@ static void test_printf(const void *test_data)
assert(l_string_length(str) == strlen("Foobar7100BarFoo"));
- a = l_string_free(str, false);
+ a = l_string_unwrap(str);
assert(a);
assert(!strcmp(a, "Foobar7100BarFoo"));
@@ -107,7 +107,7 @@ static void test_fixed(const void *test_data)
l_string_append_fixed(str, test->input, test->input_len);
assert(l_string_length(str) == strlen(test->expected));
- a = l_string_free(str, false);
+ a = l_string_unwrap(str);
assert(a);
assert(!strcmp(a, test->expected));
@@ -125,7 +125,7 @@ static void test_truncate(const void *test_data)
assert(!l_string_truncate(NULL, 8));
assert(l_string_truncate(str, 7));
- a = l_string_free(str, false);
+ a = l_string_unwrap(str);
assert(a);
assert(!strcmp(a, "Foobar7"));
l_free(a);
@@ -134,7 +134,7 @@ static void test_truncate(const void *test_data)
l_string_append(str, "Foobar7");
assert(l_string_truncate(str, 3));
l_string_append_c(str, '4');
- a = l_string_free(str, false);
+ a = l_string_unwrap(str);
assert(a);
assert(!strcmp(a, "Foo4"));
l_free(a);
--
2.10.2
6:50 a.m.
New subject: [PATCH 3/5] timeout: Add millisecond timeout functions, remove nanosecond functions
Nanosecond resolution was overkill, but there are still common use cases
for millisecond timer resolution.
---
ell/timeout.c | 109 +++++++++++++++++++++++++++++++++++++++++-----------------
ell/timeout.h | 8 ++---
2 files changed, 82 insertions(+), 35 deletions(-)
diff --git a/ell/timeout.c b/ell/timeout.c
index fc24d60..0935fd3 100644
--- a/ell/timeout.c
+++ b/ell/timeout.c
@@ -29,6 +29,7 @@
#include <string.h>
#include <sys/epoll.h>
#include <sys/timerfd.h>
+#include <limits.h>
#include "util.h"
#include "timeout.h"
@@ -91,31 +92,22 @@ static inline int timeout_set(int fd, unsigned int seconds, long
nanoseconds)
return timerfd_settime(fd, 0, &itimer, NULL);
}
-/**
- * l_timeout_create:
- * @seconds: timeout in seconds
- * @callback: timeout callback function
- * @user_data: user data provided to timeout callback function
- * @destroy: destroy function for user data
- *
- * Create new timeout callback handling.
- *
- * The timeout will on fire once. The timeout handling needs to be rearmed
- * with l_timeout_modify() to trigger again.
- *
- * Returns: a newly allocated #l_timeout object. On failure, the function
- * returns NULL.
- **/
-LIB_EXPORT struct l_timeout *l_timeout_create(unsigned int seconds,
- l_timeout_notify_cb_t callback,
- void *user_data, l_timeout_destroy_cb_t destroy)
+static bool convert_ms(unsigned long milliseconds, unsigned int *seconds,
+ long *nanoseconds)
{
- return l_timeout_create_with_nanoseconds(seconds, 0, callback,
- user_data, destroy);
+ unsigned long big_seconds = milliseconds / 1000;
+
+ if (big_seconds > UINT_MAX)
+ return false;
+
+ *seconds = big_seconds;
+ *nanoseconds = (milliseconds % 1000) * 1000000L;
+
+ return true;
}
/**
- * l_timeout_create_with_nanoseconds:
+ * timeout_create_with_nanoseconds:
* @seconds: number of seconds
* @nanoseconds: number of nanoseconds
* @callback: timeout callback function
@@ -124,13 +116,13 @@ LIB_EXPORT struct l_timeout *l_timeout_create(unsigned int seconds,
*
* Create new timeout callback handling.
*
- * The timeout will on fire once. The timeout handling needs to be rearmed
- * with l_timeout_modify_with_nanoseconds() to trigger again.
+ * The timeout will only fire once. The timeout handling needs to be rearmed
+ * with one of the l_timeout_modify functions to trigger again.
*
* Returns: a newly allocated #l_timeout object. On failure, the function
* returns NULL.
**/
-LIB_EXPORT struct l_timeout *l_timeout_create_with_nanoseconds(unsigned int seconds,
+static struct l_timeout *timeout_create_with_nanoseconds(unsigned int seconds,
long nanoseconds, l_timeout_notify_cb_t callback,
void *user_data, l_timeout_destroy_cb_t destroy)
{
@@ -173,6 +165,58 @@ LIB_EXPORT struct l_timeout
*l_timeout_create_with_nanoseconds(unsigned int seco
}
/**
+ * l_timeout_create:
+ * @seconds: timeout in seconds
+ * @callback: timeout callback function
+ * @user_data: user data provided to timeout callback function
+ * @destroy: destroy function for user data
+ *
+ * Create new timeout callback handling.
+ *
+ * The timeout will only fire once. The timeout handling needs to be rearmed
+ * with one of the l_timeout_modify functions to trigger again.
+ *
+ * Returns: a newly allocated #l_timeout object. On failure, the function
+ * returns NULL.
+ **/
+LIB_EXPORT struct l_timeout *l_timeout_create(unsigned int seconds,
+ l_timeout_notify_cb_t callback,
+ void *user_data, l_timeout_destroy_cb_t destroy)
+{
+ return timeout_create_with_nanoseconds(seconds, 0, callback,
+ user_data, destroy);
+}
+
+/**
+ * l_timeout_create_ms:
+ * @milliseconds: timeout in milliseconds
+ * @callback: timeout callback function
+ * @user_data: user data provided to timeout callback function
+ * @destroy: destroy function for user data
+ *
+ * Create new timeout callback handling.
+ *
+ * The timeout will only fire once. The timeout handling needs to be rearmed
+ * with one of the l_timeout_modify functions to trigger again.
+ *
+ * Returns: a newly allocated #l_timeout object. On failure, the function
+ * returns NULL.
+ **/
+LIB_EXPORT struct l_timeout *l_timeout_create_ms(unsigned long milliseconds,
+ l_timeout_notify_cb_t callback,
+ void *user_data, l_timeout_destroy_cb_t destroy)
+{
+ unsigned int seconds;
+ long nanoseconds;
+
+ if (!convert_ms(milliseconds, &seconds, &nanoseconds))
+ return NULL;
+
+ return timeout_create_with_nanoseconds(seconds, nanoseconds, callback,
+ user_data, destroy);
+}
+
+/**
* l_timeout_modify:
* @timeout: timeout object
* @seconds: timeout in seconds
@@ -197,15 +241,14 @@ LIB_EXPORT void l_timeout_modify(struct l_timeout *timeout,
}
/**
- * l_timeout_modify_with_nanoseconds:
+ * l_timeout_modify_ms:
* @timeout: timeout object
- * @seconds: number of seconds
- * @nanoseconds: number of nanoseconds
+ * @milliseconds: number of milliseconds
*
* Modify an existing @timeout and rearm it.
**/
-LIB_EXPORT void l_timeout_modify_with_nanoseconds(struct l_timeout *timeout,
- unsigned int seconds, long nanoseconds)
+LIB_EXPORT void l_timeout_modify_ms(struct l_timeout *timeout,
+ unsigned long milliseconds)
{
if (unlikely(!timeout))
return;
@@ -213,8 +256,12 @@ LIB_EXPORT void l_timeout_modify_with_nanoseconds(struct l_timeout
*timeout,
if (unlikely(timeout->fd < 0))
return;
- if (seconds > 0 || nanoseconds > 0) {
- if (timeout_set(timeout->fd, seconds, nanoseconds) < 0)
+ if (milliseconds > 0) {
+ unsigned int sec;
+ long nanosec;
+
+ if (!convert_ms(milliseconds, &sec, &nanosec) ||
+ timeout_set(timeout->fd, sec, nanosec) < 0)
return;
}
diff --git a/ell/timeout.h b/ell/timeout.h
index 882f166..288f0ca 100644
--- a/ell/timeout.h
+++ b/ell/timeout.h
@@ -36,13 +36,13 @@ typedef void (*l_timeout_destroy_cb_t) (void *user_data);
struct l_timeout *l_timeout_create(unsigned int seconds,
l_timeout_notify_cb_t callback,
void *user_data, l_timeout_destroy_cb_t destroy);
-struct l_timeout *l_timeout_create_with_nanoseconds(unsigned int seconds,
- long nanoseconds, l_timeout_notify_cb_t callback,
+struct l_timeout *l_timeout_create_ms(unsigned long milliseconds,
+ l_timeout_notify_cb_t callback,
void *user_data, l_timeout_destroy_cb_t destroy);
void l_timeout_modify(struct l_timeout *timeout,
unsigned int seconds);
-void l_timeout_modify_with_nanoseconds(struct l_timeout *timeout,
- unsigned int seconds, long nanoseconds);
+void l_timeout_modify_ms(struct l_timeout *timeout,
+ unsigned long milliseconds);
void l_timeout_remove(struct l_timeout *timeout);
#ifdef __cplusplus
--
2.10.2
6:50 a.m.
New subject: [PATCH 4/5] unit: Switch from nanosecond timer to millisecond timer
---
unit/test-main.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/unit/test-main.c b/unit/test-main.c
index 1f4bb31..bd68599 100644
--- a/unit/test-main.c
+++ b/unit/test-main.c
@@ -103,10 +103,8 @@ int main(int argc, char *argv[])
timeout_quit = l_timeout_create(3, timeout_quit_handler, NULL, NULL);
race_delay = l_timeout_create(1, race_delay_handler, NULL, NULL);
- race1 = l_timeout_create_with_nanoseconds(1, 100000000, race_handler,
- &race2, NULL);
- race2 = l_timeout_create_with_nanoseconds(1, 100000000, race_handler,
- &race1, NULL);
+ race1 = l_timeout_create_ms(1100, race_handler, &race2, NULL);
+ race2 = l_timeout_create_ms(1100, race_handler, &race1, NULL);
remove_self = l_timeout_create(2, remove_handler, &remove_self, NULL);
--
2.10.2
6:50 a.m.
New subject: [PATCH 5/5] unit: Add millisecond timeout overflow check
---
unit/test-main.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/unit/test-main.c b/unit/test-main.c
index bd68599..48cb16a 100644
--- a/unit/test-main.c
+++ b/unit/test-main.c
@@ -26,6 +26,8 @@
#include <ell/ell.h>
#include <unistd.h>
+#include <assert.h>
+#include <limits.h>
static void signal_handler(struct l_signal *signal, uint32_t signo,
void *user_data)
@@ -116,6 +118,12 @@ int main(int argc, char *argv[])
l_debug("hello");
+#if (ULONG_MAX > UINT_MAX)
+ l_debug("Checking timeout time limit");
+ assert(!l_timeout_create_ms((UINT_MAX + 1UL) * 1000,
+ timeout_quit_handler, NULL, NULL));
+#endif
+
l_idle_oneshot(oneshot_handler, NULL, NULL);
l_main_run();
--
2.10.2
6:59 a.m.
New subject: [PATCH 2/5] unit: Update for l_string_free changes
Hi Mat,
On 11/02/2016 04:50 PM, Mat Martineau wrote:
---
unit/test-dbus-service.c | 10 +++++-----
unit/test-string.c | 10 +++++-----
2 files changed, 10 insertions(+), 10 deletions(-)
Applied, thanks.
Regards,
-Denis
7:23 a.m.
New subject: [PATCH 3/5] timeout: Add millisecond timeout functions, remove nanosecond functions
Hi Mat,
On 11/02/2016 04:50 PM, Mat Martineau wrote:
Nanosecond resolution was overkill, but there are still common use
cases
for millisecond timer resolution.
---
ell/timeout.c | 109 +++++++++++++++++++++++++++++++++++++++++-----------------
ell/timeout.h | 8 ++---
2 files changed, 82 insertions(+), 35 deletions(-)
Patches 3-5 applied, thanks.
Regards,
-Denis
1581
days inactive
1581
days old
7 comments
2 participants
participants (2)
-
Denis Kenzior -
Mat Martineau