Return to Answer

You should first check for the number of command line arguments via argc. If there are too few, print an error message and then terminate the program.

 if (argc < 3) {
 // print an error message...
 return EXIT_FAILURE;
 }

In addition, your first existing check in main() should also return EXIT_FAILURE, instead of calling exit(1). The latter is non-portable, unlike the former, and it's already safe to return from main() if the program cannot continue execution. More info about that here.

All of your erroneous outputs shouldn't be printed to printf():

printf("ERROR: Number of cells should be integral multiple of number of threads \n");

They should instead be printed to fprintf() and stderr:

 fprintf(stderr, "Number of cells should be integral multiple of number of threads \n");

This comment is misleading:

// initialize
u[0:nx] = 0.0;
u[nx/4:nx/2] = 1.0;
f[0:nx+1] = 0.0;
res[0:nx] = 0.0;

They're not actually being initialized; they're being assigned after declaration. Initialization involves giving a variable its type and its initial value. You should do that here, which will keep variables as close in scope as possible, which is generally preferred for maintenance concerns.

Either way, you don't really need that comment. It's already clear what's being done (even after making this correction), and comments shouldn't state the obvious.

Also regarding closest possible variable scope: you have i declared towards the top of main(), but you don't use it until the for loop towards the end. If you don't have C99 (and thus cannot initialize i within the loop statement instead), declare i right before the loop.

This line doesn't look right:

td.maxthreads = omp_get_num_threads();

It appears that you're looking for omp_get_max_threads():

 td.maxthreads = omp_get_max_threads();

This will return the maximum number of available threads for a parallel environment. If this is not really your intent, then rename maxthreads to something more accurate.

You should first check for the number of command line arguments via argc. If there are too few, print an error message and then terminate the program.

 if (argc < 3) {
 // print an error message...
 return EXIT_FAILURE;
 }

In addition, your first existing check in main() should also return EXIT_FAILURE, instead of calling exit(1). The latter is non-portable, unlike the former, and it's already safe to return from main() if the program cannot continue execution. More info about that here.

All of your erroneous outputs shouldn't be printed to printf():

printf("ERROR: Number of cells should be integral multiple of number of threads \n");

They should instead be printed to fprintf() and stderr:

 fprintf(stderr, "Number of cells should be integral multiple of number of threads \n");

This comment is misleading:

// initialize
u[0:nx] = 0.0;
u[nx/4:nx/2] = 1.0;
f[0:nx+1] = 0.0;
res[0:nx] = 0.0;

They're not actually being initialized; they're being assigned after declaration. Initialization involves giving a variable its type and its initial value. You should do that here, which will keep variables as close in scope as possible, which is generally preferred for maintenance concerns.

Either way, you don't really need that comment. It's already clear what's being done (even after making this correction), and comments shouldn't state the obvious.

Also regarding closest possible variable scope: you have i declared towards the top of main(), but you don't use it until the for loop towards the end. If you don't have C99 (and thus cannot initialize i within the loop statement instead), declare i right before the loop.

This line doesn't look right:

td.maxthreads = omp_get_num_threads();

It appears that you're looking for omp_get_max_threads():

 td.maxthreads = omp_get_max_threads();

This will return the maximum number of available threads for a parallel environment. If this is not really your intent, then rename maxthreads to something more accurate.

You should first check for the number of command line arguments via argc. If there are too few, print an error message and then terminate the program.

 if (argc < 3) {
 // print an error message...
 return EXIT_FAILURE;
 }

In addition, your first existing check in main() should also return EXIT_FAILURE, instead of calling exit(1). The latter is non-portable, unlike the former, and it's already safe to return from main() if the program cannot continue execution. More info about that here.

This comment is misleading:

// initialize
u[0:nx] = 0.0;
u[nx/4:nx/2] = 1.0;
f[0:nx+1] = 0.0;
res[0:nx] = 0.0;

They're not actually being initialized; they're being assigned after declaration. Initialization involves giving a variable its type and its initial value. You should do that here, which will keep variables as close in scope as possible, which is generally preferred for maintenance concerns.

Either way, you don't really need that comment. It's already clear what's being done (even after making this correction), and comments shouldn't state the obvious.

Also regarding closest possible variable scope: you have i declared towards the top of main(), but you don't use it until the for loop towards the end. If you don't have C99 (and thus cannot initialize i within the loop statement instead), declare i right before the loop.

You should first check for the number of command line arguments via argc. If there are too few, print an error message and then terminate the program.

 if (argc < 3) {
 // print an error message...
 return EXIT_FAILURE;
 }

In addition, your first existing check in main() should also return EXIT_FAILURE, instead of calling exit(1). The latter is non-portable, unlike the former, and it's already safe to return from main() if the program cannot continue execution. More info about that here.

All of your erroneous outputs shouldn't be printed to printf():

printf("ERROR: Number of cells should be integral multiple of number of threads \n");

They should instead be printed to fprintf() and stderr:

 fprintf(stderr, "Number of cells should be integral multiple of number of threads \n");

This comment is misleading:

// initialize
u[0:nx] = 0.0;
u[nx/4:nx/2] = 1.0;
f[0:nx+1] = 0.0;
res[0:nx] = 0.0;

They're not actually being initialized; they're being assigned after declaration. Initialization involves giving a variable its type and its initial value. You should do that here, which will keep variables as close in scope as possible, which is generally preferred for maintenance concerns.

Either way, you don't really need that comment. It's already clear what's being done (even after making this correction), and comments shouldn't state the obvious.

Also regarding closest possible variable scope: you have i declared towards the top of main(), but you don't use it until the for loop towards the end. If you don't have C99 (and thus cannot initialize i within the loop statement instead), declare i right before the loop.

This line doesn't look right:

td.maxthreads = omp_get_num_threads();

It appears that you're looking for omp_get_max_threads():

 td.maxthreads = omp_get_max_threads();

This will return the maximum number of available threads for a parallel environment. If this is not really your intent, then rename maxthreads to something more accurate.

You should first check for the number of command line arguments via argc. If there are too few, print an error message and then terminate the program.

 if (argc < 3) {
 // print an error message...
 return EXIT_FAILURE;
 }

Your first check in main() should also return EXIT_FAILURE, instead of calling exit(1).

This comment is misleading:

// initialize
u[0:nx] = 0.0;
u[nx/4:nx/2] = 1.0;
f[0:nx+1] = 0.0;
res[0:nx] = 0.0;

They're not actually being initialized; they're being assigned after declaration. Initialization involves giving a variable its type and its initial value. You should do that here, which will keep variables as close in scope as possible, which is generally preferred for maintenance concerns.

Either way, you don't really need that comment. It's already clear what's being done (even after making this correction), and comments shouldn't state the obvious.

Also regarding closest possible variable scope: you have i declared towards the top of main(), but you don't use it until the for loop towards the end. If you don't have C99 (and thus cannot initialize i within the loop statement instead), declare i right before the loop.

You should first check for the number of command line arguments via argc. If there are too few, print an error message and then terminate the program.

 if (argc < 3) {
 // print an error message...
 return EXIT_FAILURE;
 }

In addition, your first existing check in main() should also return EXIT_FAILURE, instead of calling exit(1). The latter is non-portable, unlike the former, and it's already safe to return from main() if the program cannot continue execution. More info about that here.

This comment is misleading:

// initialize
u[0:nx] = 0.0;
u[nx/4:nx/2] = 1.0;
f[0:nx+1] = 0.0;
res[0:nx] = 0.0;

They're not actually being initialized; they're being assigned after declaration. Initialization involves giving a variable its type and its initial value. You should do that here, which will keep variables as close in scope as possible, which is generally preferred for maintenance concerns.

Either way, you don't really need that comment. It's already clear what's being done (even after making this correction), and comments shouldn't state the obvious.

Also regarding closest possible variable scope: you have i declared towards the top of main(), but you don't use it until the for loop towards the end. If you don't have C99 (and thus cannot initialize i within the loop statement instead), declare i right before the loop.