Avoid open filedescriptors in netcam

Description of Patch

In motion.c /***** MOTION LOOP - RETRY INITIALIZING NETCAM SECTION *****/ netcam_cleanup is called if a network camera is not available. But netcam_cleanup does not close the open socket (filedescriptor). This Patch (against snapshot motion-20060711-051001) solves this.

Installation of Patch

Download the patch file. Then copy it to the motion source directory and issue the command

patch < avoid_openfiledescriptors2.diff

Then re-build Motion and test the patch.

Change History of Patch

• 1.0 Initial revision
• 2.0 free netcam->response after netcam_disconnect

Discussion and Comments

---

Thanks for the patch. Seems OK. I have committed it. SVN 101.

-- KennethLavrsen - 13 Jul 2006

---

Not quite correct - the routine "netcam_disconnect" calls rbuf_discard, which modifies netcam->response. Your patch adds a call to netcam_disconnect after netcam->response has been freed (in netcam_cleanup).

For the other places where you change the existing (safe) sequence of "close(netcam->sock); netcam->sock=-1;" into calls to netcam_disconnect, I really don't see the advantage, and (unless you have already checked that netcam->response has previously been allocated) there may be some danger.

I suggest that, whenever you make a modification to the existing code, it's a very good idea to confirm (by running motion under the program "valgrind") that memory handling has not been adversely affected.

-- BillBrack - 13 Jul 2006

---

Peter. I reverted the SVN commit. But since you proposed the code change there must be an unresolved code problem.

Can you describe the problem you see and how you see it? We would like to fix the problem.

-- KennethLavrsen - 13 Jul 2006

---

First i changed

close(netcam->sock); netcam->sock = -1;

to

netcam_disconnect(netcam);

for a consitent look (only cosmetic)

the main fix is in netcam_cleanup.

Bill you are right that have accessed netcam->response after freeing it so i will move this free after my introduced netcam_disconnect in netcam_cleanup

-- PeterHolik - 15 Jul 2006

---

I think I didn't explain my concern with the 'cosmetic' change very clearly. By calling the routine netcam_disconnect, you are also causing a call to rbuf_discard to be made. So, it would be necessary to confirm that, at each place where you make this change, the buffer netcam->response has already been allocated (I haven't checked this personally). Note that netcam->response is only allocated for html input (i.e. not for ftp).

Hmmm... in fact, in fact, it appears that netcam_disconnect is the only place in the code where rbuf_discard is called. Perhaps a better approach would be to take that routine out of netcam_wget.c, change netcam_disconnect to check if netcam->response is non-null, and if so perform the two lines currently done in rbuf_discard. That would probably be a better fix?

-- BillBrack - 16 Jul 2006

---

rbuf_discard does the same as rbuf_initialize, also in rbuf_peek these 2 assignments are used. maybe we drop rbuf_discard and use rbuf_initialize inside of rbuf_peek.

in netcam_disconnect we can drop rbuf_discard because netcam_disconnect is only called by netcam_connect (which calls rbuf_initialize) on failure and netcam_read_html_jpeg for nonstreaming cams and netcam_cleanup (where we free rbuf).

-- PeterHolik - 17 Jul 2006

---

OK, that looks quite ok now. I have committed your proposed changes to SVN - thanks!

-- BillBrack - 17 Jul 2006

---

in 3.2.7

-- KennethLavrsen - 20 Oct 2006

---