[CTDB] Various Patches from Red Hat [Patch 3/7]

[Amitay Isaacs]

On Sat, Jun 15, 2013 at 8:05 AM, Christopher R. Hertel <crh at samba.org>wrote:

> On 06/14/2013 10:11 AM, Jose Rivera wrote:
>
>> Hello Michael, Amitay,
>>
>> Attached are a variety of patches from Red Hat, courtesy of Sumit Bose.
>> They
>> include:
>>
>>   * A collection of suggestions from Coverity
>>   * A memory leak fix
>>   * An error check
>>
>> And more. All but the Coverity-based patch are relatively small changes.
>> Since I did not author the patches themselves, I've CC'ed Sumit to answer
>> any questions you may have.
>>
>
>
> Questions on this:
>
> --- a/common/ctdb_logging.c
> +++ b/common/ctdb_logging.c
> @@ -124,7 +124,7 @@ static void ctdb_collect_log(struct ctdb_context
> *ctdb, struct ctdb_get_log_addr
>                 tm = localtime(&log_entries[tmp_**entry].t.tv_sec);
>                 strftime(tbuf, sizeof(tbuf)-1,"%Y/%m/%d %H:%M:%S", tm);
>
> -               if (log_entries[tmp_entry].**message) {
> +               if (log_entries[tmp_entry].**message[0] != '\0') {
>                         count += fprintf(f, "%s:%s %s", tbuf,
> get_debug_by_level(log_**entries[tmp_entry].level),
> log_entries[tmp_entry].**message);
>                 }
>
> In general, you want both tests (test whether the pointer is non-NULL and
> also test whether the string is non-empty).  Should it be:
>
>                 if ((log_entries[tmp_entry].**message) &&
>                     (log_entries[tmp_entry].**message[0] != '\0')) {
>                         count += fprintf(f, "%s:%s %s", tbuf,
> get_debug_by_level(log_**entries[tmp_entry].level),
> log_entries[tmp_entry].**message);
>
> ??
> I don't know what safeguards are on log_entries[].message.
>
>
In this case, ctdb_log_entry structure has a fixed size element 'message'.
So the above patch is okay.

Amitay.