dtc: Return code of fdt_get_path()

The documentation for fdt_get_path() says it returns zero on success, but it actually returns the length of the string. Is there any preference on which one to fix?

Quoted 3 lines · Scott Wood

The documentation for fdt_get_path() says it returns zero on success, but it actually returns the length of the string. Is there any preference on which one to fix?

The documentation I think. There may be existing users, and the length of the path is potentially useful.

Although.. I think the confusion came because at one stage I planned to have it return the full length of the path, even if the buffer was smaller. That would let the caller allocate a buffer of exactly the right size on a second call. Which is potentially useful, but not compatible with returning NOSPACE as we do now.

Plus.. looking over that code again, I think there may be more problems there - I think there may be cases where it runs out of space but returns BADOFFSET or some error code instead of NOSPACE.

Ugh, yes, confirmed that it is buggy. Not sure of the best way to fix this, I can see three ways to go here.

1. Minimally fix the error code bug, make sure NOSPACE is
returned when the buffer is too small, length of path otherwise, update documentation to match.
2. Make the code match the documentation: return 0 on success,
NOSPACE if the buffer is too small.
3. Update both code and documentation to match the semantics I
think I must have intended at some point: always return the path length, even if it exceeds the buffer length; caller is responsible for checking if the path in the buffer is truncated.

(3) has the possibly useful property that you can use the call with a small buffer and if it fails the caller now knows how big a buffer it must allocate. With (1) and (2) the caller's only way to deal with NOSPACE is to keep allocating bigger buffers until the call succeeds. However it makes it more complex for the caller to check the return value - not just that it's non-negative put also that it's less than the buffer size. Plus its arguably stylistically inconsistent with the read-write functions which return NOSPACE when the buffer is exceeded with no indication of how much space is needed.

Any views, Jon?

Quoted 23 lines · David Gibson

Ugh, yes, confirmed that it is buggy. Not sure of the best way to fix this, I can see three ways to go here.

1. Minimally fix the error code bug, make sure NOSPACE is
returned when the buffer is too small, length of path otherwise, update documentation to match.
2. Make the code match the documentation: return 0 on success,
NOSPACE if the buffer is too small.
3. Update both code and documentation to match the semantics I
think I must have intended at some point: always return the path length, even if it exceeds the buffer length; caller is responsible for checking if the path in the buffer is truncated.

(3) has the possibly useful property that you can use the call with a small buffer and if it fails the caller now knows how big a buffer it must allocate. With (1) and (2) the caller's only way to deal with NOSPACE is to keep allocating bigger buffers until the call succeeds. However it makes it more complex for the caller to check the return value - not just that it's non-negative put also that it's less than the buffer size. Plus its arguably stylistically inconsistent with the read-write functions which return NOSPACE when the buffer is exceeded with no indication of how much space is needed.

Quoted 1 line

Any views, Jon?

Just rolling back into town and catching up some....

Has this issue been resolved yet?

Quoted 14 lines · David Gibson

Quoted 3 lines · Scott Wood

The documentation for fdt_get_path() says it returns zero on success, but it actually returns the length of the string. Is there any preference on which one to fix?

The documentation I think. There may be existing users, and the length of the path is potentially useful.

Although.. I think the confusion came because at one stage I planned to have it return the full length of the path, even if the buffer was smaller. That would let the caller allocate a buffer of exactly the right size on a second call. Which is potentially useful, but not compatible with returning NOSPACE as we do now.

Quoted 3 lines

Plus.. looking over that code again, I think there may be more problems there - I think there may be cases where it runs out of space but returns BADOFFSET or some error code instead of NOSPACE.

Ugh, yes, confirmed that it is buggy. Not sure of the best way to fix this, I can see three ways to go here.

1. Minimally fix the error code bug, make sure NOSPACE is
returned when the buffer is too small, length of path otherwise, update documentation to match.
2. Make the code match the documentation: return 0 on success,
NOSPACE if the buffer is too small.
3. Update both code and documentation to match the semantics I
think I must have intended at some point: always return the path length, even if it exceeds the buffer length; caller is responsible for checking if the path in the buffer is truncated.

(3) has the possibly useful property that you can use the call with a small buffer and if it fails the caller now knows how big a buffer it must allocate. With (1) and (2) the caller's only way to deal with NOSPACE is to keep allocating bigger buffers until the call succeeds. However it makes it more complex for the caller to check the return value - not just that it's non-negative put also that it's less than the buffer size. Plus its arguably stylistically inconsistent with the read-write functions which return NOSPACE when the buffer is exceeded with no indication of how much space is needed.