Comments on new resolv.c additions

Daniel Mack zonque at gmail.com
Sun Sep 11 20:50:10 UTC 2011


Hi Denys,

thanks for your comments.

On Sat, Sep 10, 2011 at 3:03 AM, Denys Vlasenko
<vda.linux at googlemail.com> wrote:
> -void res_close(void)
> +static void
> +__res_iclose(res_state statp, int free_addr)
>  {
>        __UCLIBC_MUTEX_LOCK(__resolv_lock);
>        __close_nameservers();
> @@ -3552,6 +3566,26 @@ void res_close(void)
>        memset(&_res, 0, sizeof(_res));
>        __UCLIBC_MUTEX_UNLOCK(__resolv_lock);
>  }
>
> Obviously, neither statp nor free_addr parameters are used. Why are they there?

[...]

> and since __res_iclose() ignores its parameters,
> these two functions are essentially the same. Is it intended?
> If yes, can we just alias their names?

I copied most of this code from other libc-implementations and tried
to find a balance between simplifying the code to match the internal
structure of uClibc and care for similarity to the original
implementation. I might have overlooked obious places where function
wrappers could be omitted, and you seem to have hit one of them. I'd
appreciate if you came up with a proper patch to fix this :)

> -extern struct _ns_flagdata _ns_flagdata[];
> +extern const struct _ns_flagdata _ns_flagdata[];
>
> This table is used exactly in one place. You can make it static
> (and smaller).

I had quite some trouble getting this detail straight, I remember. The
particular problem here is that ns_flagdata[] is accesses as an
exernal array from applications, as the helper macros just dereference
values from it. Hence, it can not be static, unfortunately.

> +                               if (!isxdigit(c&0xff))
> +                                       return (EINVAL);
> +                               value <<= 4;
> +                               value += digitvalue[(int)c];
>
> Eek... there are much more compact ways to convert hex to binary.

Indeed.

> +                       if ((bitlen = *(lp + 1)) == 0)
>
> It's not necessary to cram as much as possible into one line.
> The assignment can well be done as a separate statement, and code
> will be easier to read. gcc will generate the same code anyway.

For these two as well: can you write up a patch to fix it up? The
commits you quoted have been applied already, so we need fix-ups
anyway.



Thanks,
Daniel


More information about the uClibc mailing list