headers
Gisle Vanem | 10 Aug 2012 13:17
Picon
Favicon
Gravatar

adig.c calls perror()

adig.c calls perror() when select() fail. This doesn't work on
Winsock; perror() prints errno string, not WSAGetLastError().
So I suggest we add a new ares_perror() function to ares_strerror.c:

--- Git-latest\ares_strerror.c  Wed Oct 12 11:43:31 2011
+++ ares_strerror.c     Fri Aug 10 13:01:10 2012
 <at>  <at>  -54,3 +54,17  <at>  <at> 
   else
     return "unknown";
 }
+
+/* perror() doesn't show Winsock errors.
+ */
+void ares_perror (const char *str)
+{
+#ifdef USE_WINSOCK
+  if (str && *str)
+     fprintf (stderr, "%s: ", str);
+  fprintf (stderr, "Winsock error %d\n", SOCKERRNO);
+#else
+  perror (str);
+#endif
+}
+

And a prototype for it in ares.h:

--- Git-latest\ares.h   Sun Jun 24 21:37:50 2012
+++ ares.h      Fri Aug 10 12:58:25 2012
 <at>  <at>  -547,6 +547,8  <at>  <at>
(Continue reading)

Permalink | Reply |
headers
Tommie Gannert | 13 Aug 2012 16:00

Re: adig.c calls perror()

2012/8/10 Gisle Vanem <gvanem@...>:
> adig.c calls perror() when select() fail. This doesn't work on
> Winsock; perror() prints errno string, not WSAGetLastError().
> So I suggest we add a new ares_perror() function to ares_strerror.c:

Makes sense to me.

Quickly and unscientifically searching the web gave me

  http://tangentsoft.net/wskfaq/articles/bsd-compatibility.html

which states the same thing.

--

-- 
Tommie Gannert
Software engineer, backend infrastructure
Spotify AB
Permalink | Reply |
headers
Daniel Stenberg | 14 Aug 2012 23:20
Picon
Favicon
Gravatar

Re: adig.c calls perror()

On Fri, 10 Aug 2012, Gisle Vanem wrote:

> adig.c calls perror() when select() fail. This doesn't work on Winsock; 
> perror() prints errno string, not WSAGetLastError(). So I suggest we add a 
> new ares_perror() function to ares_strerror.c:

Do we really need that function as part of the c-ares API? I realize that 
example is wrong, but is that reason enough to introduce a new function?

Are there others in favor of adding this function?

--

-- 

  / daniel.haxx.se
Permalink | Reply |
headers
Ben Greear | 14 Aug 2012 23:23
Favicon

Re: adig.c calls perror()

On 08/14/2012 02:20 PM, Daniel Stenberg wrote:
> On Fri, 10 Aug 2012, Gisle Vanem wrote:
>
>> adig.c calls perror() when select() fail. This doesn't work on Winsock; perror() prints errno string,
not WSAGetLastError(). So I suggest we add a new
>> ares_perror() function to ares_strerror.c:
>
> Do we really need that function as part of the c-ares API? I realize that example is wrong, but is that reason
enough to introduce a new function?
>
> Are there others in favor of adding this function?

I think the library shouldn't print anything out.  It could instead return
some error code and/or string that the application could deal with as
it sees fit.

But, I don't care that much either way :)

Thanks,
Ben

--

-- 
Ben Greear <greearb@...>
Candela Technologies Inc  http://www.candelatech.com
Permalink | Reply |
headers
Tommie Gannert | 15 Aug 2012 08:56

Re: adig.c calls perror()

2012/8/14 Daniel Stenberg <daniel@...>:
> On Fri, 10 Aug 2012, Gisle Vanem wrote:
>
>> adig.c calls perror() when select() fail. This doesn't work on Winsock;
>> perror() prints errno string, not WSAGetLastError(). So I suggest we add a
>> new ares_perror() function to ares_strerror.c:
>
>
> Do we really need that function as part of the c-ares API? I realize that
> example is wrong, but is that reason enough to introduce a new function?
>
> Are there others in favor of adding this function?

I don't think it should be public, but printing the right error code
is a good idea. :)

I also agree with Ben that the best thing would be to not print stuff
at all, but that feels like a larger change (not just fixing a bug.)

--

-- 
Tommie
Permalink | Reply |
headers
Gisle Vanem | 15 Aug 2012 12:24
Picon
Favicon
Gravatar

Re: adig.c calls perror()

"Daniel Stenberg" <daniel@...> wrote:

> Do we really need that function as part of the c-ares API? I realize that 
> example is wrong, but is that reason enough to introduce a new function?

We could probably just drop perror() in adig.c. And just print
the error number from SOCKERRNO. Otherwise (to be complete)
we should get the correct text like get_winsock_error() does
for libcurl. Maybe it's too much work, but it would be nice to
copy/edit curl_strerror.c to ares_strerror.c.

--gv
Permalink | Reply |
headers
Daniel Stenberg | 21 Aug 2012 14:21
Picon
Favicon
Gravatar

Re: adig.c calls perror()

On Wed, 15 Aug 2012, Gisle Vanem wrote:

>> Do we really need that function as part of the c-ares API? I realize that 
>> example is wrong, but is that reason enough to introduce a new function?
>
> We could probably just drop perror() in adig.c. And just print the error 
> number from SOCKERRNO.

I quite agree. Care to provide a patch?

--

-- 

  / daniel.haxx.se
Permalink | Reply |
headers
Gisle Vanem | 21 Aug 2012 17:55
Picon
Favicon
Gravatar

Re: adig.c calls perror()

"Daniel Stenberg" <daniel@...> wrote:

>> We could probably just drop perror() in adig.c. And just print the error 
>> number from SOCKERRNO.
> 
> I quite agree. Care to provide a patch?

Okay:

diff -u3 -Hb -r Git-latest\adig.c .\adig.c
--- Git-latest\adig.c   Wed Oct 12 11:43:30 2011
+++ .\adig.c    Tue Aug 21 17:51:13 2012
 <at>  <at>  -390,9 +390,9  <at>  <at> 
         break;
       tvp = ares_timeout(channel, NULL, &tv);
       count = select(nfds, &read_fds, &write_fds, NULL, tvp);
-      if (count < 0 && SOCKERRNO != EINVAL)
+      if (count < 0 && (status = SOCKERRNO) != EINVAL)
         {
-          perror("select");
+          printf("select fail: %d", status);
           return 1;
         }
       ares_process(channel, &read_fds, &write_fds);

--gv
Permalink | Reply |

Gmane