On Fri, Dec 17, 2010 at 12:19 PM, Bram Moolenaar wrote:
>
> Xavier de Gaye wrote:
>
>> 2010/12/14 Dominique Pellé wrote:
>> >
>> > clang static analyzer complains with the following warning:
>> >
>> > netbeans.c:329:6: warning: Value stored to 'sd' is never read
>> > sd = mch_open(hostname, O_RDONLY, 0);
>> > ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> >
>> > ....
>> >
>> > File descriptor sd is leaked at line 329.
>> > Why is this file being opened at line 329 without being used?
>> > Shouldn't line 326 to 331 be removed?
>> >
>>
>>
>> Hi Dominique,
>>
>> There is possibly a confusion between 'hostname' the parameter, and
>> the file from where this parameter may be read (:help
>> netbeans-parameters). Maybe the author wanted initially to print the
>> file content as a debugging help while he was writting this function ?
>> I agree with you that these lines should be removed.
>>
>> Another problem is that 'sd' is mostly never closed in case of error.
>>
>> I don't know also what is the purpose of reopening a new socket, after
>> the first connect has failed with ECONNREFUSED. This does not seem
>> useful.
>
> Using hostname as a file name was apparently used for debugging. I
> don't see how you can do much with this, and since the file descriptor
> is assigned to "sd" instead of "nbsock" it doesn't actually work.
> Let's remove these lines.
I see... I did replace, in a previous patch, 'sd' with 'nbsock' as the
global static file descriptor, so I am responsible for this leak, I
guess. This debugging method probably never worked: in vim-7.0001
before this patch, 'haveConnection' is not set to TRUE when 'hostname'
is used as the debugging input file.
> Not leaking when there is an error somewhere would also be good.
> Also, there is a "sleep(5)" that doesn't look right. Can one get out of
> that loop by pressing CTRL-C?
The ':nbstart' command can be interrupted with CTRL-C, but 'gvim -nb'
cannot be interrupted (and never could). The attached patch fixes
this and the leaking of file descriptors on error.
--
Xavier
Les Chemins de Lokoti: http://lokoti.alwaysdata.net
--
You received this message from the "vim_dev" maillist.
Do not top-post! Type your reply below the text you are replying to.
For more information, visit http://www.vim.org/maillist.php
diff --git a/src/netbeans.c b/src/netbeans.c
--- a/src/netbeans.c
+++ b/src/netbeans.c
@@ -325,6 +325,7 @@
{
nbdebug(("error in gethostbyname() in netbeans_connect()\n"));
PERROR("gethostbyname() in netbeans_connect()");
+ sock_close(sd);
goto theend;
}
memcpy((char *)&server.sin_addr, host->h_addr, host->h_length);
@@ -374,15 +375,12 @@
|| (errno == EINTR)))
{
nbdebug(("retrying...\n"));
- sleep(5);
- if (!doabort)
+ mch_delay(5000L, TRUE);
+ ui_breakcheck();
+ if (got_int)
{
- ui_breakcheck();
- if (got_int)
- {
- errno = EINTR;
- break;
- }
+ errno = EINTR;
+ break;
}
if (connect(sd, (struct sockaddr *)&server,
sizeof(server)) == 0)
@@ -397,6 +395,7 @@
/* Get here when the server can't be found. */
nbdebug(("Cannot connect to Netbeans #2\n"));
PERROR(_("Cannot connect to Netbeans #2"));
+ sock_close(sd);
if (doabort)
getout(1);
goto theend;
@@ -407,6 +406,7 @@
{
nbdebug(("Cannot connect to Netbeans\n"));
PERROR(_("Cannot connect to Netbeans"));
+ sock_close(sd);
if (doabort)
getout(1);
goto theend;
