[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] Add gn_phonebook2vcardstr()

From: Pawel Kot
Subject: Re: [PATCH] Add gn_phonebook2vcardstr()
Date: Sun, 15 Jun 2008 21:43:57 +0200


On Fri, Jun 13, 2008 at 23:32, Bastien Nocera <address@hidden> wrote:
> On Fri, 2008-06-13 at 20:47 +0200, Pawel Kot wrote:
>> Hi.
>> On Fri, Jun 13, 2008 at 00:33, Bastien Nocera <address@hidden> wrote:
>> > But here's a patch to add a function to transform phonebook entries into
>> > a string, instead of just pushing it to a file descriptor.
>> [...]
>> > Comments?
>> Yes, a few.
>> First general one. I don't think we need both. Having this one we
>> probably could:
>> fprintf(f, "%s", gn_phonebook2vcardstr(entry));
>> So we should probably give up the old one
> I didn't want to break ABI compat.

Fair enough. However could you please mark it to be removed in the
next libgnokii major version?

>> +static void gn_vcard_append_printf(vcard_string *str, const char *fmt, ...)
>> If it's static it doesn't need to be gn_. And yes, I know other static
>> functions in there are prefixed with gn_.
> :) I prefer prefixing, but I don't really mind if functions names are
> changed.

The original idea was to prefix just public functions so they were
easily recognized.

>> +GNOKII_API char * gn_phonebook2vcardstr(gn_phonebook_entry *entry)
>> +{
>> +     vcard_string str;
>> [...]
>> +     return str.str;
>> Isn't returning pointer to a local variable a bit dangerous?
> str.str is allocated on the heap, only str is on the stack.


> The main question is:
> - Do you want to keep the old function

See above.

> - What name do you want it to have

Current one seems fine.

take care,
Fred Allen  - "Imitation is the sincerest form of television."

reply via email to

[Prev in Thread] Current Thread [Next in Thread]