From: SF/projects/mingw n. l. <min...@li...> - 2011-08-21 20:23:07
|
Patches item #3395852, was opened at 2011-08-21 20:23 Message generated for change (Tracker Item Submitted) made by keithmarshall You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=302435&aid=3395852&group_id=2435 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: runtime Group: Patch needs review Status: Open Resolution: None Priority: 5 Private: No Submitted By: Keith Marshall (keithmarshall) Assigned to: Keith Marshall (keithmarshall) Summary: libmingwex.a: dirent.[ch] implementation issues Initial Comment: To facilitate implementation of a globbing capability, within an embedded script interpreter for mingw-get, I initially set out to add support for the BSD d_type field in our existing dirent implementation. The infrastructure needed to support this additional field is already present within the implementation -- the _findfirst() and _findnext() calls used to read directory content also return the required attribute data -- making addition of the feature fairly trivial. However, on inspection of the actual code, I observe the following deficiencies:-- 1) The DIR and _WDIR data types, declared in dirent.h *should* be opaque data types, yet this implementation exposes all of the gory internal detail of the implementation, (which is also rather ugly, due to grossly inefficient use of memory). 2) Although ostensibly a POSIX compliant API, this implementation fails POSIX compliance in various aspects, most notably in incorrect use of errno. 3) The efficiency of the implementation is significantly degraded, as a consequence of performing certain of the underlying system calls within the wrong dirent API functions. To remedy this, I've prepared the attached four stage cumulative patch set, delivering the following features:-- Stage 1: makes DIR and _WDIR data types opaque, as they should be. Stage 2: rationalises memory usage within DIR and _WDIR, adding the d_type field to their associated struct dirent and struct _wdirent aggregates, with identification for BSD's DT_REG, DT_DIR and DT_UNKNOWN attributes. Stage 3: corrects the misuse of errno. Stage 4: refactors the code, to perform underlying system calls within more appropriate API function scope. I propose to apply these patches in sequence, at intervals over the coming weeks. Okay to commit stage 1 after (say) seven days, to allow for preliminary review? ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=302435&aid=3395852&group_id=2435 |
From: SF/projects/mingw n. l. <min...@li...> - 2011-08-22 15:57:54
|
Patches item #3395852, was opened at 2011-08-21 16:23 Message generated for change (Comment added) made by ir0nh34d You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=302435&aid=3395852&group_id=2435 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: runtime Group: Patch needs review Status: Open Resolution: None Priority: 5 Private: No Submitted By: Keith Marshall (keithmarshall) Assigned to: Keith Marshall (keithmarshall) Summary: libmingwex.a: dirent.[ch] implementation issues Initial Comment: To facilitate implementation of a globbing capability, within an embedded script interpreter for mingw-get, I initially set out to add support for the BSD d_type field in our existing dirent implementation. The infrastructure needed to support this additional field is already present within the implementation -- the _findfirst() and _findnext() calls used to read directory content also return the required attribute data -- making addition of the feature fairly trivial. However, on inspection of the actual code, I observe the following deficiencies:-- 1) The DIR and _WDIR data types, declared in dirent.h *should* be opaque data types, yet this implementation exposes all of the gory internal detail of the implementation, (which is also rather ugly, due to grossly inefficient use of memory). 2) Although ostensibly a POSIX compliant API, this implementation fails POSIX compliance in various aspects, most notably in incorrect use of errno. 3) The efficiency of the implementation is significantly degraded, as a consequence of performing certain of the underlying system calls within the wrong dirent API functions. To remedy this, I've prepared the attached four stage cumulative patch set, delivering the following features:-- Stage 1: makes DIR and _WDIR data types opaque, as they should be. Stage 2: rationalises memory usage within DIR and _WDIR, adding the d_type field to their associated struct dirent and struct _wdirent aggregates, with identification for BSD's DT_REG, DT_DIR and DT_UNKNOWN attributes. Stage 3: corrects the misuse of errno. Stage 4: refactors the code, to perform underlying system calls within more appropriate API function scope. I propose to apply these patches in sequence, at intervals over the coming weeks. Okay to commit stage 1 after (say) seven days, to allow for preliminary review? ---------------------------------------------------------------------- >Comment By: Chris Sutcliffe (ir0nh34d) Date: 2011-08-22 11:57 Message: Sounds like a good approach. I've reviewed patch 1 and I am fine with it. ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=302435&aid=3395852&group_id=2435 |
From: SF/projects/mingw n. l. <min...@li...> - 2011-08-27 20:30:37
|
Patches item #3395852, was opened at 2011-08-21 20:23 Message generated for change (Comment added) made by keithmarshall You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=302435&aid=3395852&group_id=2435 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: runtime Group: Patch needs review Status: Open Resolution: None Priority: 5 Private: No Submitted By: Keith Marshall (keithmarshall) Assigned to: Keith Marshall (keithmarshall) Summary: libmingwex.a: dirent.[ch] implementation issues Initial Comment: To facilitate implementation of a globbing capability, within an embedded script interpreter for mingw-get, I initially set out to add support for the BSD d_type field in our existing dirent implementation. The infrastructure needed to support this additional field is already present within the implementation -- the _findfirst() and _findnext() calls used to read directory content also return the required attribute data -- making addition of the feature fairly trivial. However, on inspection of the actual code, I observe the following deficiencies:-- 1) The DIR and _WDIR data types, declared in dirent.h *should* be opaque data types, yet this implementation exposes all of the gory internal detail of the implementation, (which is also rather ugly, due to grossly inefficient use of memory). 2) Although ostensibly a POSIX compliant API, this implementation fails POSIX compliance in various aspects, most notably in incorrect use of errno. 3) The efficiency of the implementation is significantly degraded, as a consequence of performing certain of the underlying system calls within the wrong dirent API functions. To remedy this, I've prepared the attached four stage cumulative patch set, delivering the following features:-- Stage 1: makes DIR and _WDIR data types opaque, as they should be. Stage 2: rationalises memory usage within DIR and _WDIR, adding the d_type field to their associated struct dirent and struct _wdirent aggregates, with identification for BSD's DT_REG, DT_DIR and DT_UNKNOWN attributes. Stage 3: corrects the misuse of errno. Stage 4: refactors the code, to perform underlying system calls within more appropriate API function scope. I propose to apply these patches in sequence, at intervals over the coming weeks. Okay to commit stage 1 after (say) seven days, to allow for preliminary review? ---------------------------------------------------------------------- >Comment By: Keith Marshall (keithmarshall) Date: 2011-08-27 20:30 Message: Okay. On the basis of Chris' explicit approval, and accepting silence as tacit approval from others, I've committed the stage 1 patch. I'll leave it a while, before moving on to stage 2. ---------------------------------------------------------------------- Comment By: Chris Sutcliffe (ir0nh34d) Date: 2011-08-22 15:57 Message: Sounds like a good approach. I've reviewed patch 1 and I am fine with it. ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=302435&aid=3395852&group_id=2435 |
From: SF/projects/mingw n. l. <min...@li...> - 2011-10-01 20:32:45
|
Patches item #3395852, was opened at 2011-08-21 20:23 Message generated for change (Comment added) made by keithmarshall You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=302435&aid=3395852&group_id=2435 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: runtime Group: Patch needs review Status: Open Resolution: None Priority: 5 Private: No Submitted By: Keith Marshall (keithmarshall) Assigned to: Keith Marshall (keithmarshall) Summary: libmingwex.a: dirent.[ch] implementation issues Initial Comment: To facilitate implementation of a globbing capability, within an embedded script interpreter for mingw-get, I initially set out to add support for the BSD d_type field in our existing dirent implementation. The infrastructure needed to support this additional field is already present within the implementation -- the _findfirst() and _findnext() calls used to read directory content also return the required attribute data -- making addition of the feature fairly trivial. However, on inspection of the actual code, I observe the following deficiencies:-- 1) The DIR and _WDIR data types, declared in dirent.h *should* be opaque data types, yet this implementation exposes all of the gory internal detail of the implementation, (which is also rather ugly, due to grossly inefficient use of memory). 2) Although ostensibly a POSIX compliant API, this implementation fails POSIX compliance in various aspects, most notably in incorrect use of errno. 3) The efficiency of the implementation is significantly degraded, as a consequence of performing certain of the underlying system calls within the wrong dirent API functions. To remedy this, I've prepared the attached four stage cumulative patch set, delivering the following features:-- Stage 1: makes DIR and _WDIR data types opaque, as they should be. Stage 2: rationalises memory usage within DIR and _WDIR, adding the d_type field to their associated struct dirent and struct _wdirent aggregates, with identification for BSD's DT_REG, DT_DIR and DT_UNKNOWN attributes. Stage 3: corrects the misuse of errno. Stage 4: refactors the code, to perform underlying system calls within more appropriate API function scope. I propose to apply these patches in sequence, at intervals over the coming weeks. Okay to commit stage 1 after (say) seven days, to allow for preliminary review? ---------------------------------------------------------------------- >Comment By: Keith Marshall (keithmarshall) Date: 2011-10-01 20:32 Message: Stage 1 has now been in play for more than a month, with no negative reports. Therefore, I've gone ahead with stage 2, which I'll leave for another two or three weeks before proceeding to stage 3. ---------------------------------------------------------------------- Comment By: Keith Marshall (keithmarshall) Date: 2011-08-27 20:30 Message: Okay. On the basis of Chris' explicit approval, and accepting silence as tacit approval from others, I've committed the stage 1 patch. I'll leave it a while, before moving on to stage 2. ---------------------------------------------------------------------- Comment By: Chris Sutcliffe (ir0nh34d) Date: 2011-08-22 15:57 Message: Sounds like a good approach. I've reviewed patch 1 and I am fine with it. ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=302435&aid=3395852&group_id=2435 |
From: SF/projects/mingw n. l. <min...@li...> - 2012-03-10 20:30:14
|
Patches item #3395852, was opened at 2011-08-21 13:23 Message generated for change (Comment added) made by keithmarshall You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=302435&aid=3395852&group_id=2435 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: MinGW runtime >Group: IINR - Include In Next Release >Status: Closed >Resolution: Accepted Priority: 5 Private: No Submitted By: Keith Marshall (keithmarshall) Assigned to: Keith Marshall (keithmarshall) Summary: libmingwex.a: dirent.[ch] implementation issues Initial Comment: To facilitate implementation of a globbing capability, within an embedded script interpreter for mingw-get, I initially set out to add support for the BSD d_type field in our existing dirent implementation. The infrastructure needed to support this additional field is already present within the implementation -- the _findfirst() and _findnext() calls used to read directory content also return the required attribute data -- making addition of the feature fairly trivial. However, on inspection of the actual code, I observe the following deficiencies:-- 1) The DIR and _WDIR data types, declared in dirent.h *should* be opaque data types, yet this implementation exposes all of the gory internal detail of the implementation, (which is also rather ugly, due to grossly inefficient use of memory). 2) Although ostensibly a POSIX compliant API, this implementation fails POSIX compliance in various aspects, most notably in incorrect use of errno. 3) The efficiency of the implementation is significantly degraded, as a consequence of performing certain of the underlying system calls within the wrong dirent API functions. To remedy this, I've prepared the attached four stage cumulative patch set, delivering the following features:-- Stage 1: makes DIR and _WDIR data types opaque, as they should be. Stage 2: rationalises memory usage within DIR and _WDIR, adding the d_type field to their associated struct dirent and struct _wdirent aggregates, with identification for BSD's DT_REG, DT_DIR and DT_UNKNOWN attributes. Stage 3: corrects the misuse of errno. Stage 4: refactors the code, to perform underlying system calls within more appropriate API function scope. I propose to apply these patches in sequence, at intervals over the coming weeks. Okay to commit stage 1 after (say) seven days, to allow for preliminary review? ---------------------------------------------------------------------- >Comment By: Keith Marshall (keithmarshall) Date: 2012-03-10 12:30 Message: I've now committed all four patch sets -- the last one five weeks ago -- with no adverse (or indeed any) feedback. This completes this task, hence I'm closing this ticket. ---------------------------------------------------------------------- Comment By: Keith Marshall (keithmarshall) Date: 2011-10-01 13:32 Message: Stage 1 has now been in play for more than a month, with no negative reports. Therefore, I've gone ahead with stage 2, which I'll leave for another two or three weeks before proceeding to stage 3. ---------------------------------------------------------------------- Comment By: Keith Marshall (keithmarshall) Date: 2011-08-27 13:30 Message: Okay. On the basis of Chris' explicit approval, and accepting silence as tacit approval from others, I've committed the stage 1 patch. I'll leave it a while, before moving on to stage 2. ---------------------------------------------------------------------- Comment By: Chris Sutcliffe (ir0nh34d) Date: 2011-08-22 08:57 Message: Sounds like a good approach. I've reviewed patch 1 and I am fine with it. ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=302435&aid=3395852&group_id=2435 |