how to make the below code look better

[Steven D'Aprano]

On Wed, 2 Dec 2015 11:11 pm, Ganesh Pal wrote:

> Hello team,
> 
> I need suggestion to improve the below code , Iam on Linux and python 2.7
> 
> if not os.path.ismount("/tmp"):
>            sys.exit("/tmp not mounted.")

This is good enough for quick and dirty scripts, but this is vulnerable to a
race condition. It may be that /tmp is mounted *now*, but a millisecond
later (before you can use it) another process unmounts it.

This is called a "time of check to time of use" bug:

https://cwe.mitre.org/data/definitions/367.html

https://www.owasp.org/index.php/Time_of_check,_time_of_use_race_condition

https://en.wikipedia.org/wiki/Time_of_check_to_time_of_use

and can be a serious software vulnerability.

If this code is only being used under trusted conditions, then it is
probably okay, otherwise you should reconsider your strategy.

(Besides, how often do you unmount /tmp?)




> else:
>      if  create_dataset() and check_permission():

this with:

elif create_dataset() and check_permission():

but again be mindful of the risk of race conditions.



>      try:
>           run_full_back_up()
>           run_partial_back_up()
>     except Exception, e:
>             logging.error(e)
>             sys.exit("Running backup failed")
>     if not validation_errors():
>         sys.exit("Validation failed")

I would swap the sense of validation_errors(). Currently, it looks like it
returns True if there are *no* errors, and False if there are errors. That
seems confusing and easy to get wrong. 

    if validation_errors():
        sys.exit("Validation failed")

reads better and is easier to understand. Now validation_errors() returns
true if there are errors, and false if there are no errors.


>     else:
>         try:
>             compare_results()
>         except Exception, e:
>                logging.error(e)
>                sys.exit("Comparing result failed")
> 
> Question 1:
> 
> 1.  if  create_dataset() and check_permission():
>     Iam assuming that if statement will be executed only if both the
> functions are true ?

Correct.


> 2. Can I have a if statement within  if else ? , some how I feel its messy

Yes you can, but elif is usually nicer, and saves a level of indentation.



-- 
Steven