On Fri, Jun 16, 2006 at 01:36:01PM +0200, Peter Troeger wrote:
The DRMAA specification is intended to be language independent, and hopefully talks always about the length of a string. A string is a collection of printable characters, therefore '\0' should not count. Could it be that you talk here about specifics of the C-binding ("error_diag_len", "job_id_len") ?
Yes, I'm talking mainly about the C-binding, but the specifiaction also adds some words about _BUFFER constants and is sometimes inconsistent as you noted... The constants (and whether they include '\0' or not) is one issue, but I think the most important is what all the _len arguments mean in C-binding. The worst thing that could happen when a developer misunderstands the constants (max len vs. buffer size) is truncation, but a wrong argument in function calls could lead to a crash. From C-binding document: job_id_len - The size in characters of the job identifier buffer. This would clearly mean sizeof(buf), but the "characters" makes me think it could be the maximum number of printable characters that will be put into the buffer. In my opionion this word is not needed here, it should only say "the size of the job identifier buffer". And as for the constants, in my opinion, as they are supposed to be langunage independent, they should mean the max string length... but then, the name preffix _MAXLEN instead of _BUFFER would be better.
String length vs. buffer size would be optimal.
indeed.
I'm more convinced that the constants define the total buffer size, but some DRMAA example source codes got me confused, e.g. they define variables char foo[DRMAA_JOBNAME_BUFFER] but use DRMAA_JOBNAME_BUFFER - 1 as call arguments.
I think it was intended to do it the first way. I propose that the DRMAA spec should say:
"The length of any output context-specific error string SHALL NOT exceed DRMAA_ERROR_STRING_BUFFER - 1."
But doesn't it make the specification language dependent? -- Piotr Domagalski
On Fri, Jun 16, 2006 at 05:17:37PM +0200, Piotr Domagalski wrote:
a wrong argument in function calls could lead to a crash. From C-binding document:
job_id_len - The size in characters of the job identifier buffer.
This would clearly mean sizeof(buf), but the "characters" makes me think it could be the maximum number of printable characters that will be put into the buffer. In my opionion this word is not needed here, it should only say "the size of the job identifier buffer".
... and as I see in SGE's DRMAA implementation[1], all the _len arguments are treated as maximum string lengths, thus the input buffer is always considered to have size of (job_id_len + 1). So, to sum up, should implementations treat these arguments as size of the buffer or the maximum string length that would be put into the buffer? --------------------- [1] By the way: some comments in the code are a bit misleading: OUTPUTS char *job_id - buffer for resulting jobid size_t job_id_len - size of job_id buffer char *error_diagnosis - diagnosis buffer size_t error_diag_len - diagnosis buffer length The "size of job_id buffer" seems completly wrong, whereas "buffer length" is not a clear wording from the C language perspective. -- Piotr Domagalski
This one is for the SGE bug tracker ?
... and as I see in SGE's DRMAA implementation[1], all the _len arguments are treated as maximum string lengths, thus the input buffer is always considered to have size of (job_id_len + 1).
So, to sum up, should implementations treat these arguments as size of the buffer or the maximum string length that would be put into the buffer?
[1] By the way: some comments in the code are a bit misleading:
OUTPUTS char *job_id - buffer for resulting jobid size_t job_id_len - size of job_id buffer char *error_diagnosis - diagnosis buffer size_t error_diag_len - diagnosis buffer length
The "size of job_id buffer" seems completly wrong, whereas "buffer length" is not a clear wording from the C language perspective.
The overall naming issue is now filed as tracker item: https://forge.gridforum.org/sf/go/artf5493?nav=1 Peter.
participants (2)
-
Peter Troeger
-
Piotr Domagalski