linux-btrfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Goffredo Baroncelli <kreijack@tiscalinet.it>
To: Wang Shilong <wangshilong1991@gmail.com>
Cc: "Goffredo Baroncelli" <goffredo.baroncelli@yahoo.com>,
	linux-btrfs@vger.kernel.org, "Hugo Mills" <hugo@carfax.org.uk>,
	"Michael Kjörling" <michael@kjorling.se>,
	"Martin Steigerwald" <Martin@lichtvoll.de>,
	cwillu <cwillu@cwillu.com>,
	"Chris Murphy" <lists@colorremedies.com>,
	"David Sterba" <dsterba@suse.cz>, "Zach Brown" <zab@zabbo.net>,
	"Goffredo Baroncelli" <kreijack@inwind.it>
Subject: Re: [PATCH 1/8] Add some helpers to manage the strings allocation/deallocation.
Date: Sun, 10 Mar 2013 15:51:07 +0100	[thread overview]
Message-ID: <513C9DDB.4030804@tiscalinet.it> (raw)
In-Reply-To: <9199E774-B2A4-4C7F-B85A-464BF00FA517@gmail.com>

On 03/10/2013 03:34 PM, Wang Shilong wrote:
> Hello,
[...]
>> +
>> +/* 
>> + * Add a string to the dynamic allocated string list
>> + */
>> +char *string_list_add(char *s)
>> +{
>> +	int  size;
>> +
> 	
> 	I'd prefer to have a check here firstly, like:
> 	if (!s)
> 		return s;
> 
> 	Since this function is called directly without any check about 'char *s'
>         in your next patch..
> 	
> 	Thanks,
> 	Wang

Thanks for the review.
However this check is not mandatory. Even if s == null, we store a null
pointer which is meaningless but not dangerous because it is legal to
free() a null pointer.

Form man 3 free:

	void free(void *ptr);
	[...]
	If ptr is NULL, no operation is performed.

In a next submit I will add your suggestions.
> 
>> +	size = sizeof(void *) * ++count_string_to_free;
>> +	strings_to_free = realloc(strings_to_free, size);
>> +
>> +	/* if we don't have enough memory, we have more serius
>> +	   problem than that a wrong handling of not enough memory */
>> +	if (!strings_to_free) {
>> +		fprintf(stderr, "add_string_to_free(): Not enough memory\n");
>> +		count_string_to_free = 0;
>> +		return NULL;
>> +	}
>> +
>> +	strings_to_free[count_string_to_free-1] = s;
>> +	return s;
>> +}
>> +
>> +/*
>> + * Free the dynamic allocated strings list
>> + */
>> +void string_list_free()
>> +{
>> +	int	i;
>> +	for (i = 0 ; i < count_string_to_free ; i++)
>> +		free(strings_to_free[i]);
>> +
>> +	free(strings_to_free);
>> +
>> +	strings_to_free = 0;
>> +	count_string_to_free = 0;
>> +}
>> +
>> +
>> diff --git a/string_list.h b/string_list.h
>> new file mode 100644
>> index 0000000..fdc027d
>> --- /dev/null
>> +++ b/string_list.h
>> @@ -0,0 +1,23 @@
>> +/*
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public
>> + * License v2 as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> + * General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public
>> + * License along with this program; if not, write to the
>> + * Free Software Foundation, Inc., 59 Temple Place - Suite 330,
>> + * Boston, MA 021110-1307, USA.
>> + */
>> +
>> +#ifndef STRING_LIST_H
>> +#define STRING_LIST_H
>> +
>> +char *string_list_add(char *s);
>> +void string_list_free();
>> +
>> +#endif
>> -- 
>> 1.7.10.4
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 


-- 
gpg @keyserver.linux.it: Goffredo Baroncelli (kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5

  reply	other threads:[~2013-03-10 14:49 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-03-10 12:17 [PATCH V3][BTRFS-PROGS] Enhance btrfs fi df with raid5/6 support Goffredo Baroncelli
2013-03-10 12:17 ` [PATCH 1/8] Add some helpers to manage the strings allocation/deallocation Goffredo Baroncelli
2013-03-10 14:34   ` Wang Shilong
2013-03-10 14:51     ` Goffredo Baroncelli [this message]
2013-03-10 12:17 ` [PATCH 2/8] Enhance the command btrfs filesystem df Goffredo Baroncelli
2013-03-10 15:00   ` Wang Shilong
2013-03-10 12:17 ` [PATCH 3/8] Create the man page entry for the command btrfs fi df Goffredo Baroncelli
2013-03-10 12:17 ` [PATCH 4/8] Add helpers functions to handle the printing of data in tabular format Goffredo Baroncelli
2013-03-10 12:17 ` [PATCH 5/8] Add command btrfs filesystem disk-usage Goffredo Baroncelli
2013-03-10 12:17 ` [PATCH 6/8] Create entry in man page for " Goffredo Baroncelli
2013-03-10 12:17 ` [PATCH 7/8] Add btrfs device disk-usage command Goffredo Baroncelli
2013-03-10 12:17 ` [PATCH 8/8] Create a new entry in btrfs man page for btrfs device disk-usage Goffredo Baroncelli
2013-03-10 13:16 ` [PATCH V3][BTRFS-PROGS] Enhance btrfs fi df with raid5/6 support Martin Steigerwald
2013-03-10 13:19   ` Martin Steigerwald
2013-03-10 15:59     ` Goffredo Baroncelli
2013-03-10 15:52   ` Goffredo Baroncelli
2013-03-21 15:27 ` Bart Noordervliet
  -- strict thread matches above, loose matches on Subject: below --
2013-02-23 13:46 [PATCH V2][BTRFS-PROGS] " Goffredo Baroncelli
2013-02-23 13:46 ` [PATCH 1/8] Add some helpers to manage the strings allocation/deallocation Goffredo Baroncelli
2013-02-25  2:20   ` Eric Sandeen
2013-02-25 19:59     ` Goffredo Baroncelli
2013-02-25 20:19       ` Zach Brown
2013-02-25 21:00         ` Goffredo Baroncelli
2013-02-18 21:04 [PATCH][BTRFS-PROGS] Enhance btrfs fi df with raid5/6 support Goffredo Baroncelli
2013-02-18 21:04 ` [PATCH 1/8] Add some helpers to manage the strings allocation/deallocation Goffredo Baroncelli
2013-02-18 23:08   ` Zach Brown
2013-02-19  7:07     ` Goffredo Baroncelli
2013-02-19 17:21       ` Zach Brown
2013-02-19 17:44         ` Goffredo Baroncelli
2013-02-19 17:59           ` Zach Brown
2013-02-19 21:28             ` Goffredo Baroncelli
2013-02-19 21:40               ` Zach Brown

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=513C9DDB.4030804@tiscalinet.it \
    --to=kreijack@tiscalinet.it \
    --cc=Martin@lichtvoll.de \
    --cc=cwillu@cwillu.com \
    --cc=dsterba@suse.cz \
    --cc=goffredo.baroncelli@yahoo.com \
    --cc=hugo@carfax.org.uk \
    --cc=kreijack@inwind.it \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=lists@colorremedies.com \
    --cc=michael@kjorling.se \
    --cc=wangshilong1991@gmail.com \
    --cc=zab@zabbo.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).