[Lldb-commits] [PATCH] D25247: Make LLDB -Werror clean under clang

[Adrian McCarthy via lldb-commits]

amccarth accepted this revision.
amccarth added a comment.

lgtm



> UDPSocket.cpp:106
> +#if defined(_MSC_VER) && defined(UNICODE)
> +        "getaddrinfo(%s, %s, &hints, &info) returned error %i (%S)",
> +#else

Yuck.  Given that this is going to get reduced from UTF-16 to MBCS, it might be cleaner to leave the format string alone and call gai_strerrorA explicitly on Windows.  I guess it would be just as ugly that way.

> SelectHelper.cpp:122
> +      updateMaxFd(max_error_fd, fd);
> +    updateMaxFd(max_fd, fd);
>    }

I find this logic less clear than the original version, since it's not obvious that `updateMaxFd` uses an in-out parameter until you go look up to see what it does.  It also seems weird to have an output parameter come before an input-only parameter.

Perhaps if `updateMaxFd` returned the value instead of updating the parameter, it would be more obvious:

  max_fd = updateMaxFd(max_fd, fd);

https://reviews.llvm.org/D25247