This patch makes several changes.
1) Change the behavior of path_concat so that it will always return an empty string if it hits ENOBUFS. 2) Clean up the code, adding more comments. 3) Modify the unit tests to check for empty string and to make sure we don't write past the allowed buffer if the input strings are too long. 4) Update the doxygen for path_concat so that it's clear that path will be empty on failure.
On Wed, Mar 07, 2012 at 10:08:29AM -0500, Stephen Gallagher wrote:
This patch makes several changes.
- Change the behavior of path_concat so that it will always return an
empty string if it hits ENOBUFS. 2) Clean up the code, adding more comments. 3) Modify the unit tests to check for empty string and to make sure we don't write past the allowed buffer if the input strings are too long. 4) Update the doxygen for path_concat so that it's clear that path will be empty on failure.
- if (!path || path_size < 1) return ENOBUFS;
- if (!path || path_size < 1) {
ret = ENOBUFS;- }
You just return with ENOBUFS here, because nothing else can be done. Please add unit test where path is NULL and path_size is zero to check the (small) path as well.
bye, Sumit
On Wed, 2012-03-07 at 16:23 +0100, Sumit Bose wrote:
On Wed, Mar 07, 2012 at 10:08:29AM -0500, Stephen Gallagher wrote:
This patch makes several changes.
- Change the behavior of path_concat so that it will always return an
empty string if it hits ENOBUFS. 2) Clean up the code, adding more comments. 3) Modify the unit tests to check for empty string and to make sure we don't write past the allowed buffer if the input strings are too long. 4) Update the doxygen for path_concat so that it's clear that path will be empty on failure.
- if (!path || path_size < 1) return ENOBUFS;
- if (!path || path_size < 1) {
ret = ENOBUFS;- }
You just return with ENOBUFS here, because nothing else can be done. Please add unit test where path is NULL and path_size is zero to check the (small) path as well.
Thanks for the review. Fixed.
On Wed, Mar 07, 2012 at 10:31:00AM -0500, Stephen Gallagher wrote:
On Wed, 2012-03-07 at 16:23 +0100, Sumit Bose wrote:
On Wed, Mar 07, 2012 at 10:08:29AM -0500, Stephen Gallagher wrote:
This patch makes several changes.
- Change the behavior of path_concat so that it will always return an
empty string if it hits ENOBUFS. 2) Clean up the code, adding more comments. 3) Modify the unit tests to check for empty string and to make sure we don't write past the allowed buffer if the input strings are too long. 4) Update the doxygen for path_concat so that it's clear that path will be empty on failure.
- if (!path || path_size < 1) return ENOBUFS;
- if (!path || path_size < 1) {
ret = ENOBUFS;- }
You just return with ENOBUFS here, because nothing else can be done. Please add unit test where path is NULL and path_size is zero to check the (small) path as well.
Thanks for the review. Fixed.
ACK
bye, Sumit
On Wed, 2012-03-07 at 21:55 +0100, Sumit Bose wrote:
On Wed, Mar 07, 2012 at 10:31:00AM -0500, Stephen Gallagher wrote:
On Wed, 2012-03-07 at 16:23 +0100, Sumit Bose wrote:
On Wed, Mar 07, 2012 at 10:08:29AM -0500, Stephen Gallagher wrote:
This patch makes several changes.
- Change the behavior of path_concat so that it will always return an
empty string if it hits ENOBUFS. 2) Clean up the code, adding more comments. 3) Modify the unit tests to check for empty string and to make sure we don't write past the allowed buffer if the input strings are too long. 4) Update the doxygen for path_concat so that it's clear that path will be empty on failure.
- if (!path || path_size < 1) return ENOBUFS;
- if (!path || path_size < 1) {
ret = ENOBUFS;- }
You just return with ENOBUFS here, because nothing else can be done. Please add unit test where path is NULL and path_size is zero to check the (small) path as well.
Thanks for the review. Fixed.
ACK
Pushed to ding-libs master and ding_libs-0-1
sssd-devel@lists.fedorahosted.org
-
Stephen Gallagher -
Sumit Bose