[PATCH v2] make function rtnl_link_set_name() more reasonable

[Teto]

Would it be too much work or introduce inconsistent behaviors if we
checked only when DEBUG is defined ? like:
int rtnl_link_set_name(struct rtnl_link *link, const char *name)
{
int err = 0;
#ifdef DEBUG
if (strlen(name) >= IFNAMSIZ)
   err = -1;
#endif

strncpy(link->l_name, name, sizeof(link->l_name) - 1);
link->ce_mask |= LINK_ATTR_IFNAME;

return err;
}

2014/1/22 Hongwei Bi <hwbi2008 at gmail.com>:
> Hi Thomas,
>
> The full check is overkill surely. I think there are two method we can
> do to solve it.
>
> first: we don't tune the code. when the length of name is equal or
> greater then 16, we just notify the user that  the length has been
> truncated to 15 bytes by means of modifying the code comment.
>
> second:  tune the code as follows:
>
> int rtnl_link_set_name(struct rtnl_link *link, const char *name)
> {
> int err = 0;
> if (strlen(name) >= IFNAMSIZ)
>    err = -1;
>
> strncpy(link->l_name, name, sizeof(link->l_name) - 1);
> link->ce_mask |= LINK_ATTR_IFNAME;
>
> return err;
> }
>
> If you have any other suggests, please let me know. Thanks.
>
> Hongwei
>
>
> 2014/1/22 Thomas Haller <thaller at redhat.com>:
>> Hi Hongwei,
>>
>> sorry, re-thinking this again... :)
>>
>> The users ~should~ provide valid arguments.
>> If they don't, it's basically a bug on their side and libnl is generally
>> not forgiving about that (just note, that most functions don't do any
>> input validation or range checking).
>>
>> If you want to validate the ifname, shouldn't you also check that the
>> whole name is valid? Something like
>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/net/core/dev.c?id=d8ec26d7f8287f5788a494f56e8814210f0e64be#n929
>>
>> Otherwise the return value, just tells you, "yeah, it's maybe
>> valid" -- which isn't much use. And the full check I think is overkill.
>>
>> What do you guys think?
>>
>>
>> Thomas
>>
>>
>>
>> On Wed, 2014-01-15 at 22:47 +0800, Hongwei Bi wrote:
>>> We set the maximum value of IFNAMSIZ is 16 bytes.So when the length of
>>> the new link's name is equal or greater then 16,we should notify the user
>>> that the length of the name has been truncated to 15 bytes.
>>>
>>> Signed-off-by: Hongwei Bi <hwbi2008 at gmail.com>
>>> ---
>>>  include/netlink/route/link.h |  2 +-
>>>  lib/route/link.c             | 19 ++++++++++++++++++-
>>>  2 files changed, 19 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/include/netlink/route/link.h b/include/netlink/route/link.h
>>> index 3345787..566e224 100644
>>> --- a/include/netlink/route/link.h
>>> +++ b/include/netlink/route/link.h
>>> @@ -151,7 +151,7 @@ extern int        rtnl_link_str2carrier(const char *);
>>>  extern void  rtnl_link_set_qdisc(struct rtnl_link *, const char *);
>>>  extern char *        rtnl_link_get_qdisc(struct rtnl_link *);
>>>
>>> -extern void  rtnl_link_set_name(struct rtnl_link *, const char *);
>>> +extern int   rtnl_link_set_name(struct rtnl_link *, const char *);
>>>  extern char *        rtnl_link_get_name(struct rtnl_link *);
>>>
>>>  extern void  rtnl_link_set_group(struct rtnl_link *, uint32_t);
>>> diff --git a/lib/route/link.c b/lib/route/link.c
>>> index 9979b5b..9b2b561 100644
>>> --- a/lib/route/link.c
>>> +++ b/lib/route/link.c
>>> @@ -1651,11 +1651,28 @@ void rtnl_link_put(struct rtnl_link *link)
>>>   * @route_doc{link_attr_name, Link Name}
>>>   * @see rtnl_link_get_name()
>>>   * @see rtnl_link_set_ifindex()
>>> + *
>>> + *@return -1 when the length of the name is equal or greater than IFNAMSIZ.
>>> + *      In this case,the length of the name has been truncated to 15 bytes.
>>> + *@return 0 normally
>>>   */
>>> -void rtnl_link_set_name(struct rtnl_link *link, const char *name)
>>> +int rtnl_link_set_name(struct rtnl_link *link, const char *name)
>>>  {
>>> +     int err = 0;
>>> +     if (!name)
>>> +             goto errout;
>>> +     else if (strlen(name) == 0)
>>> +             goto errout;
>>> +     else if (IFNAMSIZ > strlen(name))
>>> +             err = 0;
>>> +     else
>>> +             err = -1;
>>> +
>>>       strncpy(link->l_name, name, sizeof(link->l_name) - 1);
>>>       link->ce_mask |= LINK_ATTR_IFNAME;
>>> +
>>> +     errout:
>>> +             return err;
>>>  }
>>>
>>>  /**
>>
>
> _______________________________________________
> libnl mailing list
> libnl at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/libnl