linux-btrfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Goffredo Baroncelli <kreijack@gmail.com>
To: Zach Brown <zab@zabbo.net>
Cc: kreijack@inwind.it, 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>
Subject: Re: [PATCH 1/8] Add some helpers to manage the strings allocation/deallocation.
Date: Tue, 19 Feb 2013 08:07:06 +0100	[thread overview]
Message-ID: <5123249A.6080502@gmail.com> (raw)
In-Reply-To: <20130218230842.GN22221@lenny.home.zabbo.net>

On 02/19/2013 12:08 AM, Zach Brown wrote:
> On Mon, Feb 18, 2013 at 10:04:26PM +0100, Goffredo Baroncelli wrote:
>> This patch adds some helpers to manage the strings allocation and
>> deallocation.
>> The function string_list_add(char *) adds the passed string to a list;
>> the function string_list_free() frees all the strings together.
> 
> Please don't do this.  To verify that a given pointer isn't freed before
> it's used we'd have to make sure that there are no string_list_free()
> calls in the interim that would hit their pointer on this global list.

The idea is that the code shouldn't care about do deallocate the strings
until finishing its jobs. At the end it calls string_list_free().

Of course, if after string_list_free() some dynamically allocated
strings are used then bad things could happen.

Ideally string_list_free() should be called at the end of the main.
I don't think that btrfs-progs allocates an huge quantity of string, so
this could be an acceptable behaviour.

> 
> As far as I can tell, this is only used for the pretty units?  Instead
> of
> 	printf("%s", leaked_string(raw));
> 
> how about
> 
> 	printf("%llu%s", scaled_value(raw), static_unit_str(raw));
> 
> That'd avoid the need to pass back arbitrary allocated strings and this
> code could go away.

Sorry I don't understand the differences between {leaked, scaled,
raw}_string. Could you elaborate a bit ?

> 
>> +	if (!strings_to_free) {
>> +		fprintf(stderr, "add_string_to_free(): Not enough memory\n");
>> +		strings_to_free = 0;
> 
> if (a == 0) a = 0?
> 
>> +		count_string_to_free = 0;
>> +	}
>> +
>> +	strings_to_free[count_string_to_free-1] = s;
> 
> NULL[-1] = s?

Right, I will correct soon.

> 
> - z
> --
> 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-02-19  7:13 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
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
2013-02-18 21:04 ` [PATCH 2/8] Enhance the command btrfs filesystem df Goffredo Baroncelli
2013-02-18 21:04 ` [PATCH 3/8] Create the man page entry for the command btrfs fi df Goffredo Baroncelli
2013-02-18 21:04 ` [PATCH 4/8] Add helpers functions to handle the printing of data in tabular format Goffredo Baroncelli
2013-02-18 21:04 ` [PATCH 5/8] Add command btrfs filesystem disk-usage Goffredo Baroncelli
2013-02-18 21:04 ` [PATCH 6/8] Create entry in man page for " Goffredo Baroncelli
2013-02-18 21:04 ` [PATCH 7/8] Add btrfs device disk-usage command Goffredo Baroncelli
2013-02-18 21:04 ` [PATCH 8/8] Create a new entry in btrfs man page for btrfs device disk-usage Goffredo Baroncelli
  -- strict thread matches above, loose matches on Subject: below --
2013-02-23 13:46 [PATCH V2][BTRFS-PROGS] Enhance btrfs fi df with raid5/6 support 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-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

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=5123249A.6080502@gmail.com \
    --to=kreijack@gmail.com \
    --cc=Martin@lichtvoll.de \
    --cc=cwillu@cwillu.com \
    --cc=dsterba@suse.cz \
    --cc=hugo@carfax.org.uk \
    --cc=kreijack@inwind.it \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=lists@colorremedies.com \
    --cc=michael@kjorling.se \
    --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).