videowhisper [videowhisper]

The archive is not in the correct format. The archive should contain a directory which is named after the short name of the project (as you plan to set it); that directory must contain all the files for the module. You can check the structure of an archive for an already existing Drupal project.

Bear in mind that files under a license different from GPL License are not allowed in Drupal CVS; the same is true for files that are available from third-party web sites.

1. The indentation used should be two spaces; coding standards suggest to not use tab characters.
2. The installation code delete all the content of the cache table; it should remove only the rows that interest it, but then it doesn't make sense to do it during the installation.

3.         'fields' => array(
               'id' => array('type' => 'serial', 'size' => 'big', 'unsigned' => TRUE, 'not null' => TRUE,
   

   Isn't the field site too big? Compare the definition of that field with the primary key for the nodes table.

       'fields' => array(
         'nid' => array(
           'description' => 'The primary identifier for a node.',
           'type' => 'serial',
           'unsigned' => TRUE,
           'not null' => TRUE),
   

   The node primary key is handling a lot of nodes in Drupal.org, but it doesn't still create any problems.

4.   //Don't put a comma after primary key definition, since doing so will cause database errors.
   

   The error is caused by the schema array that is not correctly defined (the keys are not defined in the fields).
   Compare the schema you defined with the following one, used by Drupal core code (which uses the comma after the last key value, and it doesn't create any database errors):

     $schema['variable'] = array(
       'description' => 'Named variable/value pairs created by Drupal core or any other module or theme. All variables are cached in memory at the start of every Drupal request so developers should not be careless about what is stored here.',
       'fields' => array(
         'name' => array(
           'description' => 'The name of the variable.',
           'type' => 'varchar',
           'length' => 128,
           'not null' => TRUE,
           'default' => ''),
         'value' => array(
           'description' => 'The value of the variable.',
           'type' => 'text',
           'not null' => TRUE,
           'size' => 'big'),
         ),
       'primary key' => array('name'),
       );
   

5. /**
   * Access to chat room page
   * @returns true by default 
   */
   
   function v2wvc_roomaccess($arg)
   {
   	return true;
   }
   

   The function just returns TRUE in any cases, and it doesn't use the argument passed to it. If you want to allow the access to the user with the permission "use 2wayvideochat", simply remove the access callback index.
   'access callback' => v2wvc_roomaccess is not correct, anyway.

   Also, follow the Drupal coding standards for all the code.

6.   '#title' => t('New Room Name')
   

   Use the capital case only for the first word; the reported title should be "New room name".

7.   '#default_value' => 65536
   

   Setting the default as you do will always show that value, whatever value has been previously selected by the user. Read more about Form API Reference, and Form API Quickstart GUide. Check also the examples reported there.

8. The module should implement hook_requirements if it is not able to run when it doesn't found a file the user must download.
9. I think that the chat rooms should be created as nodes, and the module changed. See How to create a content (node) type.

A few points (ran out of time):

v2wvc_delete is not a hook_delete implementation. Best to rename this function to prevent possible conflicts.

v2wvc_delete is vulnerable to CSRF. If you make this a confirmation form and do the delete in its submission handler, the issue is solved.

return t('Room not found or does not belong to you.')." ($room by $username)";

$room by $username is not 1) translatable, 2) $username is not escaped. Please use placeholders and see http://drupal.org/node/28984 and http://drupal.org/node/263002.

$myrooms = db_query("SELECT * FROM {v2wvc_rooms} WHERE user='".$user->name."' ORDER BY timelastaccess DESC");

Please use placeholders. Mrs. O'Reilly might once use your module.

l( $row[room] ...

As l() runs check_plain on plain text titles, you'll see double-escaped roomnames (Foo&Baz's Bar instead of Foo&Baz's Bar)

Other variables in v2wvc_new may need escaping.

See the Drupal coding standards to understand how a module code should be written.

1. Check the IF-statements.
2. Check the string-concatenation operator.

3.     db_query("UPDATE {v2wvc_rooms} SET room='%s', welcome='%s', timecreated='%d', timelastaccess='%d', timeexpire='%d', timeout='%d', credits='%d', bandwidth='%d', maxbandwidth='%d', camwidth='%d', camheight='%d', camfps='%d', visitors='%d' WHERE vid = %d", $room, $welcome, $ztime, $ztime, $expiration, $node->advanced['cleanup'] * 3600, $node->advanced['credits'], $node->advanced['webcam']['bandwidth'], $node->advanced['webcam']['maxbandwidth'], $cam[0], $cam[1], $node->advanced['webcam']['camfps'], $node->advanced['visitors'], $node->vid);
   

   %d is for integers, which don't need the quotation marks.

4. LICENSE.txt must be removed.

  else $found = t('Please <a target="_blank" href="http://www.videowhisper.com/?p=Drupal+2+Way+Video+Chat+Module">download 2 Way Video Chat application</a>. Application folder NOT detected: ') . $path;

should be rewritten as

  else $found = t('Please <a target="_blank" href="@url-download">download 2 Way Video Chat application</a>. Application folder NOT detected: %path', array('@url-download' => 'http://www.videowhisper.com/?p=Drupal+2+Way+Video+Chat+Module', '%path' => $path));