Re: RESEND [wininet]add implementation of few more options

[Dmitry Timoshkov]

"Vijay Kiran Kamuju" [e-mail] wrote:
> as previous implementation had some bugs use this version
> -
>    case INTERNET_OPTION_VERSION:
>    {
> -
>        if (*lpdwBufferLength < sizeof(HTTP_VERSION_INFO))
> -
>            INTERNET_SetLastError(ERROR_INSUFFICIENT_BUFFER);
> -
>        /*
> -
>         * Presently hardcoded to 1.2
> -
>         */
> -
>        if(lpBuffer != NULL)
> -
>        {
> -
>            ((INTERNET_VERSION_INFO*)lpBuffer)->dwMajorVersion = 1;
> -
>            ((INTERNET_VERSION_INFO*)lpBuffer)->dwMinorVersion = 2;

You still need to check buffer size before filling the buffer. Also using
a new variable instead of casting lpBuffer would be much cleaner. Same
applies to all cases.
> -
>    case INTERNET_OPTION_OFFLINE_MODE:
> -
>    case INTERNET_OPTION_OFFLINE_SEMANTICS:
> -
>    case INTERNET_OPTION_POLICY:
> -
>    case INTERNET_OPTION_RECEIVE_THROUGHPUT:
> -
>    case INTERNET_OPTION_SEND_THROUGHPUT:
> -
>        WARN("This request is not supported by native either\n");
> -
>        SetLastError(ERROR_NOT_SUPPORTED);
> -
>        break;

I'd suggest to use FIXME instead of WARN here. It doesn't really matter
that current version of miscrosoft's implementation doesn't support them,
they may add support for them later, and the apps will start to use them.
> - char useragent[] = {'W','i','n','i','n','e','t',' ','T','e','s','t',0 };

char useragent[] = "Wininet Test";

works just fine.
> - retval=InternetQueryOptionA(hinet,INTERNET_OPTION_USER_AGENT,NULL,&len);
> - err=GetLastError();

Again, checking last error without setting it first to a bogus value.

+  SetLastError(0);
+  buffer=HeapAlloc(GetProcessHeap(),0,len+1);
+  retval=InternetQueryOptionA(hinet,INTERNET_OPTION_USER_AGENT,buffer,&len);
+  err=GetLastError();

Do not set last error to 0, you then miss the case when API sets it to 0.

You don't initialize length variable before calling InternetQueryOption.

RESEND [wininet]add implementation of few more options

[Andreas Mohr]

Hi,

On Wed, Nov 09, 2005 at 12:07:57AM +0800, Dmitry Timoshkov wrote:
> "Vijay Kiran Kamuju" [e-mail] wrote:
> > - char useragent[] = {'W','i','n','i','n','e','t',' ','T','e','s','t',0 };
>
> char useragent[] = "Wininet Test";
>
> works just fine.

Maybe it works, but not "fine" ;)

static const char useragent[] = "Wininet Test";

does.

Andreas