The code without a nasty memory leak is here...


GList *
g_list_new(gpointer p, ...)
{
        gpointer s;
        va_list  argp;
        GList*   list;

        if ( p == NULL )
                return NULL;

        list = g_list_append( NULL, p );
        va_start( argp, p );
        while ( NULL != ( s = va_arg( argp, gpointer ) ) ) {
                list = g_list_append( list, s );
        }

        va_end( argp );

        return list;
}

GSList *
g_slist_new(gpointer p, ...)
{
        gpointer s;
        va_list  argp;
        GSList*  list;

        if ( p == NULL )
                return NULL;

        list = g_slist_append( NULL, p );
        va_start( argp, p );
        while ( NULL != ( s = va_arg( argp, gpointer ) ) ) {
                list = g_slist_append( list, s );
        }

        va_end( argp );

        return list;
}

Your implementations have O(n^2) complexity, you'll want to use *list_prepend inside the loop and then return *list_reverse (list);
Also adding GCC 0-sentinel attributes could be helpfull here.

You are right. For longer input it might matter quite a lot. Here is the fixed version.


GList *
g_list_new (gpointer p, ...)
{
        gpointer s;
        va_list  argp;
        GList*   list;

        if ( p == NULL )
                return NULL;

        list = g_list_prepend ( NULL, p );
        va_start ( argp, p );
        while ( NULL != ( s = va_arg ( argp, gpointer ) ) ) {
                list = g_list_prepend ( list, s );
        }

        va_end ( argp );

        return g_list_reverse ( list );
}

GSList *
g_slist_new (gpointer p, ...)
{
        gpointer s;
        va_list  argp;
        GSList*  list;

        if ( p == NULL )
                return NULL;

        list = g_slist_prepend ( NULL, p );
        va_start ( argp, p );
        while ( NULL != ( s = va_arg ( argp, gpointer ) ) ) {
                list = g_slist_prepend ( list, s );
        }

        va_end ( argp );

        return g_slist_reverse ( list );
}

(In reply to comment #3)
> You are right. For longer input it might matter quite a lot. Here is the fixed
> version.
> 
> 
> GList *
> g_list_new (gpointer p, ...)
> {
[...]

Thanks, It'd also be good if you tried to rework the code according to the GLib coding style, e.g. put spaces like "func (arg)", and add documentation comments in the usual manner (mentioning Since: 2.19, etc, there're plenty of examples in glib). Finally, please produce a patch with diff -up against GLib trunk and attach the patch file here, rather than adding raw code inside comments (which tends to bloat the reports after a few iterations).

Created attachment 117044 [details] [review]
g_list_new and g_slist_new patch

Here is the patch with gtk-doc comments.

Maybe this just adds confusion but if one is not scared of macros or Boost pre-processor library the list creation can be done like this without any overhead:

------

#include <boost/preprocessor/facilities/overload.hpp>


#define G_LIST_NEW(t,...) BOOST_PP_OVERLOAD(_G_LIST_NEW_,__VA_ARGS__)(t,__VA_ARGS__)
#define _G_LIST_NEW_0(t) {t = NULL;}
#define _G_LIST_NEW_1(t,_0) {_G_LIST_NEW_0(t) GList _t = { _0, t }; t = _g_list_alloc (); *t=_t;}
#define _G_LIST_NEW_2(t,_0,_1) {_G_LIST_NEW_1(t,_1) GList _t = { _0, t }; t = _g_list_alloc (); *t=_t;}
#define _G_LIST_NEW_3(t,_0,_1,_2) {_G_LIST_NEW_2(t,_1,_2) GList _t = { _0, t }; t = _g_list_alloc (); *t=_t;}
#define _G_LIST_NEW_4(t,_0,_1,_2,_3) {_G_LIST_NEW_3(t,_1,_2,_3) GList _t = { _0, t }; t = _g_list_alloc (); *t=_t;}
#define _G_LIST_NEW_5(t,_0,_1,_2,_3,_4) {_G_LIST_NEW_4(t,_1,_2,_3,_4) GList _t = { _0, t }; t = _g_list_alloc (); *t=_t;}

...

int main ()
{
    GList* list;
    ...
    G_LIST_NEW(list,a,b,c,d,e);
}

--------

But, of course, internals of GList (or a compatible type with different name) should be public. In real code, the name of temporary variable, _t, could be '_ ## t' in order to avoid conflicts. Also dependes on C99 compliance.

-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/glib/issues/122.