[netcf-devel] [PATCH 3/7] Add strlower, update dutil_linux.c:aug_match_mac

David Lutterkort lutter at redhat.com
Wed Jul 29 10:55:11 UTC 2009


On Thu, 2009-07-23 at 14:37 +0200, Jonas Eriksson wrote:
> dutil.{c,h}:
>   - Added strllower
> 
> dutil_linux.{c,h}:
>   - Made use of strllower and aug_fmt_match in aug_match_mac

Heh, I just broke that with the patch I committed. I'll rebase this and
the following patches so that the patch series still applies.

>   - Fixed a typo in the comment above the function head and prototype
> 
> drv_initscruots.c:
>   - Updated the use of aug_match_mac
>   - Broke the long function head of drv_list_interfaces

Such formatting changes should generally go into a separate patch.

> diff --git a/src/dutil.c b/src/dutil.c
> index 28832b3..2066a45 100644
> --- a/src/dutil.c
> +++ b/src/dutil.c
> @@ -62,6 +62,13 @@ int xasprintf(char **strp, const char *format, ...) {
>    return result;
>  }
>  
> +/* Convert string of max length buflen to uppercase in place */
> +void strllower(char *s, unsigned int buflen) {

I find strnlower a more natural name for this, in parallel to other
functions that only look at a limited number of characters in strings
like strndup, strncmp etc.

> +    if (s != NULL)
> +        for (int i = 0; s[i] != '\0' && i < buflen; i++)

To stick with the parallel to strndup etc., instead of a buflen, the
caller should pass in the number n of characters to look at, so that the
termination condition will be s[i] != '\0' && i <= n.

> +            s[i] = tolower(s[i]);
> +}
> +

Using tolower here is wrong, since tolower's notion of lower/uppercase
depends on the user's locale, whereas we want to always map ABCDEF ->
abcdef. That's why I used c_tolower from gnulib.
 
> diff --git a/src/dutil_linux.c b/src/dutil_linux.c
> index 4e99ef6..9727367 100644
> --- a/src/dutil_linux.c
> +++ b/src/dutil_linux.c
> @@ -39,23 +39,20 @@
>  #include "dutil.h"
>  #include "dutil_linux.h"
>  
> -/* Returns a list of all interfaces with MAC address INTF */
> +/* Returns a list of all interfaces with MAC address MAC */
>  int aug_match_mac(struct netcf *ncf, const char *mac, char ***matches) {
> -    int r, nmatches;
> -    char *path;
> -    struct augeas *aug = get_augeas(ncf);
> +    int nmatches;
> +    char *lcmac = NULL;
>  
> -    r = xasprintf(&path,
> -            "/files/sys/class/net/*[address/content = '%s']", mac);
> -    ERR_COND_BAIL(r < 0, ncf, ENOMEM);
> +    /* Make sure that the mac address is in lower case */
> +    lcmac = strndup(mac, 17);
> +    ERR_COND_BAIL(lcmac == NULL, ncf, ENOMEM);
> +    strllower(lcmac, 18);

Why do you restrict the length of lcmac here ? I'd expect the caller
makes sure that MAC is a valid MAC address, and hardcoding the length of
MAC here gains nothing and might lead to headache in the future if we
ever deal with e.g. 64bit MACs

David



More information about the netcf-devel mailing list