Am Freitag, den 24.02.2006, 14:28 +0100 schrieb Gerold J. Wucherpfennig:
Please commit.
I have no knowledge about fci, but some code is strange:
+#define fci_set_error(A,B,C) \ + p_fci_internal->perf->erfOper = A; \ + p_fci_internal->perf->erfType = B; \ + p_fci_internal->perf->fError = C; +
You use SetLastError() for B almost everywhere following the Macro. Why not add this statement to the Macro?
Exceptions where you did not use SetLastError(): Macro-Parameter B is 0 - Did you mean NO_ERROR / ERROR_SUCCESS? - Does Windows use a SetLastError(NO_ERROR) in this case? (Tests for cabinet.dll are only in one file: "extract.c". can you add tests for fci/compress?)
When windows does a SetLastError(NO_ERROR): append "SetLastError(B)" to the Macro
When windows does not do a SetLastError(NO_ERROR): append "if (B) SetLastError(B)" to the Macro
+ /* write error */ + fci_set_error( FCIERR_TEMP_FILE, ERROR_WRITE_FAULT, TRUE ); + p_fci_internal->perf->erfType = ERROR_WRITE_FAULT; + p_fci_internal->perf->fError = TRUE; + SetLastError(ERROR_WRITE_FAULT);
the two p_pci_* - lines are duplicates (already done in the Macro)
btw: What should be added to the changelog? cabinet/fci: cleanup error-handling with a MACRO
On Friday 24 February 2006 22:57, Detlef Riekenberg wrote:
Am Freitag, den 24.02.2006, 14:28 +0100 schrieb Gerold J. Wucherpfennig:
Please commit.
I have no knowledge about fci, but some code is strange:
+#define fci_set_error(A,B,C) \
- p_fci_internal->perf->erfOper = A; \
- p_fci_internal->perf->erfType = B; \
- p_fci_internal->perf->fError = C;
You use SetLastError() for B almost everywhere following the Macro. Why not add this statement to the Macro?
OK, but then there would be a SetLastError(0). I don't know if that's ok.
Exceptions where you did not use SetLastError(): Macro-Parameter B is 0
- Did you mean NO_ERROR / ERROR_SUCCESS?
- Does Windows use a SetLastError(NO_ERROR) in this case?
(Tests for cabinet.dll are only in one file: "extract.c". can you add tests for fci/compress?)
I have never used such tests I don't know. And I have very big problems in doing the compression there are two libraries for lzx and deflate compression, but I suppose it's too difficult for me.
When windows does a SetLastError(NO_ERROR): append "SetLastError(B)" to the Macro
When windows does not do a SetLastError(NO_ERROR): append "if (B) SetLastError(B)" to the Macro
- /* write error */
- fci_set_error( FCIERR_TEMP_FILE, ERROR_WRITE_FAULT, TRUE );
- p_fci_internal->perf->erfType = ERROR_WRITE_FAULT;
- p_fci_internal->perf->fError = TRUE;
- SetLastError(ERROR_WRITE_FAULT);
the two p_pci_* - lines are duplicates (already done in the Macro)
btw: What should be added to the changelog?
cabinet/fci: cleanup error-handling with a MACRO
It's not only the macro. The patch is half a year old, so let me think about it... I think there was one bug fixed and the verbose info about the progress was completed.
Please CC me.
-
Detlef Riekenberg -
Gerold J. Wucherpfennig