On 11/29/2010 04:16 PM, Davidlohr Bueso wrote:
> On Mon, 2010-11-29 at 15:29 +0100, Marek Polacek wrote:
>> Hi Dave,
>>
>> On 11/29/2010 03:16 PM, Davidlohr Bueso wrote:
>>> From: Davidlohr Bueso <dave@xxxxxxx>
>>>
>>> This notation is a little more user friendly and applies to information about removable, readonly and rotational devices
>>>
>>> Signed-off-by: Davidlohr Bueso <dave@xxxxxxx>
>>> ---
>>> misc-utils/lsblk.c | 47 ++++++++++++++++++++++++++++++++++++++---------
>>> 1 files changed, 38 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/misc-utils/lsblk.c b/misc-utils/lsblk.c
>>> index 6c5a205..a8c29a6 100644
>>> --- a/misc-utils/lsblk.c
>>> +++ b/misc-utils/lsblk.c
>>> @@ -504,6 +504,24 @@ static void probe_device(struct blkdev_cxt *cxt)
>>> return;
>>> }
>>>
>>> +static int is_removable_device(struct blkdev_cxt *cxt)
>>> +{
>>> + char *str = NULL;
>>
>> Assigning NULL is not necessary here.
>>
>>> + int ret = 0;
>>> +
>>> + str = sysfs_strdup(cxt, "removable");
>>> +
>>> + if (!str && cxt->parent)
>>> + str = sysfs_strdup(cxt->parent, "removable");
>>> +
>>> + if (str) {
>>> + ret = atoi(str);
>>
>> ato*() functions are obsolete, we probably should use strtol() instead.
>>
>
> Obsolete? Not sure about that. It doesn't detect errors, as opposed to
> strtol, but in this case str will always be either "0" or "1" so it
> should be OK to use.
>
Well, according to glibc manual, it is:
http://www.gnu.org/software/libc/manual/html_mono/libc.html#index-atoi-2567
In fact, atoi() just in turn calls strtol(). So _I_ think, in new code
we should rather use strtol(). Not that it would matter much :-).
>>> + free(str);
>>
>> free(NULL) is safe.
>>
>
> Yes, but we have to test str for NULL anyways, why not include the free
> there, once we get the ret value? This way we save an unnecessary call.
>
Right you are.
> Thanks,
> Davidlohr
>
Thanks,
Marek
--
To unsubscribe from this list: send the line "unsubscribe util-linux-ng" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
