Works for me :-)

/M

On 2013-06-17 10:25, Moe Jette wrote:
Hi Magnus,

Thank you for reporting this problem and providing a patch. Most places
that use strftime already test for a return value of zero, but I see two
that do not. Perhaps using a macro that can be used from multiple places
will be a better solution than changing the code in various places. See
attached variation of your patch.

Moe

Quoting Magnus Jonsson <[email protected]>:

Hi!

We found an issue in sacct that we pined down to a strftime call in
'src/common/parse_time.c' (slurm_make_time_str).

Reproducable with (in 2.5.{6,7}):

OK:
% sacct -Po end%19 -s failed,completed,timeout,cancelled -S 2013-06-13
| head | cat -vet
2013-06-14T11:40:14$
2013-06-14T11:40:14$
2013-06-14T11:40:11$


Fail:
sacct -Po end%18 -s failed,completed,timeout,cancelled -S 2013-06-13 |
head | cat -vet
End$
2013-06-14TVM-^P^?$
2013-06-14TVM-^P^?$
2013-06-14TVM-^P^?$

The problem is that the output from strftime with a buffer less the
the required length of output is undefined in libc since libc 4.4.4.

Different libc implementations seams to implement this differently. On
Solaris for example the buffer is truncated at the given length but
still returns 0.

From man page of strftime (Ubuntu Precise):

------8<------------
RETURN VALUE
The  strftime() function returns the number of characters placed in the
array s, not including the terminating null byte, provided the  string,
including  the  terminating  null byte, fits.  Otherwise, it returns 0,
and the contents of the array is  undefined.   (This  behavior  applies
since  at  least  libc  4.4.4;  very old versions of libc, such as libc
4.4.1, would return max if the array was too small.)

Note that the return value 0 does not necessarily  indicate  an  error;
for example, in many locales %p yields an empty string.
------8<------------

From man page of strftime (Solaris)
------8<------------
If  the total number of resulting characters including the terminating
null character is more than maxsize, strftime() returns 0 and the
contents of the array are indeterminate.
------8<------------

The return value of strftime is not checked for in slurm_make_time_str
making the returned value undefined.

As I see it the problem can be solved in many ways.

1. using a "large" temporary buffer for the output the expected
behaviour of sacct end%N will for most normal cases be fine.

2. Only checking the return value and returning an error or an well
defined output.

I haved attached a patch for case 1 with that sets the output to "#"
if the output still not fit into the buffer.

There seams to be other places in the slurm code base that uses
strftime and not checking the return code. Some of them might be OK
due to the format string and size of the buffer. But this might needs
to be looked into with more depth.

Best regards,
Magnus

--
Magnus Jonsson, Developer, HPC2N, Umeå Universitet



--
Magnus Jonsson, Developer, HPC2N, Umeå Universitet

Attachment: smime.p7s
Description: S/MIME Cryptographic Signature

Reply via email to