search

danielgtaylor / jpeg-archive

Utilities for archiving JPEGs for long term storage.
1.16k stars 121 forks source link

Checking if openning output file was sucessful. #55

Closed aidin36 closed 6 years ago

aidin36 commented 7 years ago

Resolves Issue #32 Preventing Segmentation Fault if output file doesn't exists or user does not have access to it.

lfos commented 6 years ago

Thank you for working on this. I am currently going through old PRs and working with contributors to get them merged.

There are some issues with your patch:

  1. Parts of you patch use tabs instead of spaces which breaks indentation.
  2. We usually just return 1 on errors. Is there any reason you chose 18?
aidin36 commented 6 years ago
  1. It's a good practice to return different error numbers in case of different errors. Thus, the caller process can check the status code to find out what error occured. Though, using 18 as magic number like this is not a good practice! The ideal is to have some Macros for different errors.

If you don't want to modify the behavior like this, I will change 18 to 1.

  1. Sorry. I will fix that after we decided about the second issue.
lfos commented 6 years ago

I agree that it is usually a good idea to return different values such that the caller can handle different errors accordingly. However, in this case, we (you) decided it is the callees responsibility to handle the error (i.e. display the error message) and it suffices to inform the caller about whether everything went fine or not.

aidin36 commented 6 years ago

Agreed.

Both issues fixed.

lfos commented 6 years ago

Merged with some minor whitespace changes, thanks!