Hi klausi,
Thanks for taking time to review this
I changed following things.
...

Hi klausi,

Thanks for taking time to review this

I changed following things.

1. "variable_set('allowed_extension', 'gif png jpg jpeg pdf doc xls');": all variables defined by your module need to be prefixed with your module's name to avoid name clashes with others. Same for "variable_set('attachment_id', $attachment_value);": Fixed
2. account_settings_email_attachment_enable(): No need to set variables when enabling the module since you can make use of default values with variable_get() anyway.:I did not get this point, my motive is to set a variable at time of module installation and with default value.In allowed extension I need that value to set default file extension.
3. account_settings_email_attachment_enable(): so when I disable and then re-enable the module my variable setting gets overridden?:Yes you are correct this is a wrong thing to do, fixed.
4. account_settings_email_attachment_form_alter(): if you are only targeting one form you should use hook_form_FORM_ID_alter() instead, see https://api.drupal.org/api/drupal/modules!system!system.api.php/function... :Fixed
5. "t('Click "Browse..." to select a file to upload. Allowed extension :') . str_replace(' ', ',', variable_get('allowed_extension')),": do not concatenate variables into translatable strings, use placeholders with t() instead. See https://api.drupal.org/api/drupal/includes!bootstrap.inc/function/t/7: Fixed
6. "'access arguments' => array('administer access control'),": that permission does not exist in Drupal core and it is also not defined by your module in hook_permission()? So the settings page will currently only work for user 1.:Fixed