[m-rev.] for review: handle I/O errors when reporting statistics

[Zoltan Somogyi]

On Mon, 5 Apr 2021 03:38:39 +1000 (AEST), Julien Fischer <jfischer at opturion.com> wrote:
> @@ -288,21 +238,17 @@ ML_report_standard_stats(io.MR_MercuryFileStruct stream)
>       long real_time_at_prev_stat = real_time_at_last_stat;
>       real_time_at_last_stat = System.DateTime.Now.Ticks;
> 
> -    try {
> -        io.mercury_print_string(stream, System.String.Format(
> -            ""[User time: +{0:F2}s, {1:F2}s Real time: +{2:F2}s, {3:F2}s]\\n"",
> -            (user_time_at_last_stat - user_time_at_prev_stat),
> -            (user_time_at_last_stat - user_time_at_start),
> -            ((real_time_at_last_stat - real_time_at_prev_stat)
> -                / (double) System.TimeSpan.TicksPerSecond),
> -            ((real_time_at_last_stat - real_time_at_start)
> -                / (double) System.TimeSpan.TicksPerSecond)
> -        ));
> -        // XXX At this point there should be a whole bunch of memory usage
> -        // statistics.
> -    } catch (System.SystemException e) {
> -        // XXX how should we handle I/O errors when printing statistics?
> -    }
> +    io.mercury_print_string(stream, System.String.Format(
> +        ""[User time: +{0:F2}s, {1:F2}s Real time: +{2:F2}s, {3:F2}s]\\n"",
> +        (user_time_at_last_stat - user_time_at_prev_stat),
> +        (user_time_at_last_stat - user_time_at_start),
> +        ((real_time_at_last_stat - real_time_at_prev_stat)
> +            / (double) System.TimeSpan.TicksPerSecond),
> +        ((real_time_at_last_stat - real_time_at_start)
> +           / (double) System.TimeSpan.TicksPerSecond)
> +    ));
> +    // XXX At this point there should be a whole bunch of memory usage
> +    // statistics.
>   }

This kind of change is much easier to review when the diff is done with diff -b.
As this diff is formatted now, I am just *assuming* that the stuff whose indentation
changed had no other relevant changes.

> @@ -91,23 +93,35 @@ MR_report_standard_stats(FILE *fp, int *line_number)
>       eng = MR_get_engine();
>   #endif
> 
> -    fprintf(fp, "[User time: +%.3fs, %.3fs,",
> -        (MR_user_time_at_last_stat - user_time_at_prev_stat) / 1000.0,
> -        (MR_user_time_at_last_stat - MR_user_time_at_start) / 1000.0
> -    );
> +    if (
> +        fprintf(fp, "[User time: +%.3fs, %.3fs,",
> +            (MR_user_time_at_last_stat - user_time_at_prev_stat) / 1000.0,
> +            (MR_user_time_at_last_stat - MR_user_time_at_start) / 1000.0
> +        ) < 0
> +    ) {
> +        return errno;
> +    }

Instead of putting calls to fprintf into the condition of an if-then-else,
I would *much* rather you just assigned the return value from the fprintf
to a local var, and then tested *that*, here and everywhere else.

Other than that, the diff is fine, though I do note that this diff does
not extend to checking for I/O errors when printing tabling statistics.

Zoltan.