Fantastic exchange from DConf

[Ali Çehreli via Digitalmars-d]

On 05/09/2017 10:26 PM, H. S. Teoh via Digitalmars-d wrote:
 > On Wed, May 10, 2017 at 01:32:33AM +0000, Jack Stouffer via 
Digitalmars-d wrote:
 >> On Wednesday, 10 May 2017 at 00:30:42 UTC, H. S. Teoh wrote:
 >>> 		strncpy(tmp, desc->data1, bufsz);
 >>> 		if (fwrite(tmp, strlen(tmp), 1, fp) != 1)
 >>> 		{
 >>> 			fclose(fp);
 >>> 			unlink("blah");
 >>> 			return IO_ERROR;
 >>> 		}
 >>>
 >>> 		strncpy(tmp, desc->data2, bufsz);
 >>> 		if (fwrite(tmp, strlen(tmp), 1, fp) != 1)
 >>> 		{
 >>> 			fclose(fp);
 >>> 			unlink("blah");
 >>> 			return IO_ERROR;
 >>> 		}
 >>
 >> I think you cause a memory leak in these branches because you forget
 >> to free tmp before returning.
 >
 > Well, there ya go. Case in point.

I caught that too but I thought you were testing whether we were 
listening. ;)

 > Eventually, the idiom that I (and others) eventually converged on looks
 > something like this:
 >
 > 	int myfunc(blah_t *blah, bleh_t *bleh, bluh_t *bluh) {
 > 		void *resource1, *resource2, *resource3;
 > 		int ret = RET_ERROR;
 >
 > 		/* Vet arguments */
 > 		if (!blah || !bleh || !bluh)
 > 			return ret;
 >
 > 		/* Acquire resources */
 > 		resource1 = acquire_resource(blah->blah);
 > 		if (!resource1) goto EXIT;
 >
 > 		resource2 = acquire_resource(bleh->bleh);
 > 		if (!resource1) goto EXIT;

Copy paste error! :p (resource1 should be resource2.)

 >
 > 		resource3 = acquire_resource(bluh->bluh);
 > 		if (!resource1) goto EXIT;

Ditto.

 >
 > 		/* Do actual work */
 > 		if (do_step1(blah, resource1) == RET_ERROR)
 > 			goto EXIT;
 >
 > 		if (do_step2(blah, resource1) == RET_ERROR)
 > 			goto EXIT;
 >
 > 		if (do_step3(blah, resource1) == RET_ERROR)
 > 			goto EXIT;
 >
 > 		ret = RET_OK;
 > 	EXIT:
 > 		/* Cleanup everything */
 > 		if (resource3) release(resource3);
 > 		if (resource2) release(resource2);
 > 		if (resource1) release(resource1);
 >
 > 		return ret;
 > 	}

As an improvement, consider hiding the checks and the goto statements in 
macros:

     resource2 = acquire_resource(bleh->bleh);
     exit_if_null(resource1);

     err = do_step2(blah, resource1);
     exit_if_error(err);

Or something similar... Obviously, it requires certain standardization 
like functions never having a goto statement, yet all having an EXIT 
area, etc. It makes C code very uniform, which is a good thing as you 
notice nonstandard idioms quickly.

This safer way of needing to do everything in steps of two lines is one 
of the reasons why I was convinced that exceptions are superior to 
return codes.

Ali