Motion.c Maintenance and Bugfix Patch

Introduction

This patch does not add any functionality to Motion. It simply cleans up motion.c by adding function documentation, splitting up the main function and fixing some bugs. See below for details.

Description of Patch

The patch (version 1) does the following:

• Adds a lot of documentation - function comments for all functions, comments for global variables and some additional code comments.
• Removes the storethread struct, as it wasn't used.
• Removes unnecessary braces in sig_handler and simplifies the switch statement wrt SIGHUP.
• Breaks out the destruction of struct context to a separate function, context_destroy. This is done to avoid duplicate code in the restart/quit cases, but is also related to a bugfix, see below.
• In addition, breaks out the following from main to separate functions:
  • Daemon initialization, to become_daemon.
  • Creation of cnt_list, to cntlist_create.
  • Shutdown code (used both for restart and quit), to motion_shutdown (couldn't name it only shutdown, as it would conflict with some other code).
  • Startup code (used both for restart and start), to motion_startup.
  • Signal initialization, to setup_signals.
• Simplifies the log message in myrealloc when size is 0.
• Removes a lot of duplicate code in mystrftime and puts it after the switch statement.
• Removes some redundant return statements (in functions with void as return type).
• Fixes some bugs:
  • The restart cleanup (as opposed to the quit cleanup) didn't call netcam_cleanup and didn't do config param cleanup. Now context_destroy, which takes care of this, is called.
  • In mystrftime, tempstr was never reset to point to the beginning of tempstring. This would cause a buffer overflow if enough format specifiers were present in the format string, e.g. 11 %v.

There are no functional changes, as stated above, with the following tiny exceptions:

• The netcam field isn't set to NULL when cleaning up a struct context, simply because the struct is freed right afterwards anyway.
• The log message that states thread config file has been moved out from the mutex lock around threads_running, because I didn't think it should be there. This may have the effect that the log messages don't arrive in the same order as the threads are given thread numbers, but I don't see that as a problem.

Installation of Patch

Simply:

1. Change to the Motion source directory.
2. Issue the command zcat motion-3.2.2_snap11-func.patch.gz | patch -p1.
3. Compile and test.

Change History of Patch

Version history:

• Version 1 (10 Aug 2005):
  • Adds a lot of documentation.
  • Simplifies main by moving code into separate functions.
  • Fixes a bug in strftime.
  • Fixes a bug in cleanup on restart.

Discussion and Comments

---

I originally made the patch for snap10, but it applied almost without errors to snap11 as well. Only 1 out of 68 hunks were rejected. However, the patch initially contained initialization/destruction for the global_lock mutex, but as that has been included in snap11, I had to clean up some duplicate code that resulted.

What I'm trying to say is that there may be some minor glitch due to the fact that the patch was written for snap10. I have of course double-checked the code, but you never know...

-- PerJonsson - 10 Aug 2005

---

Oh, and another thing. The patch doesn't touch motion_loop at all, but IMHO, that's a 870-line monster function that should be broken up for clarity/maintenance reasons.

-- PerJonsson - 10 Aug 2005

---

I have integrated this patch and the code will be in 3.2.2_snap12

-- KennethLavrsen - 10 Aug 2005

---