Hello, I had a look on this patch.
Let me show you some comments about the README, Makefile and
buffer_capture_cmp of the second part for the present. A
continuation of this comment would be seen later..
- contrib/buffer_capture_cmp/README
- 'contains' seems duplicate in the first paragraph.
- The second pragraph says that 'This script can use the node number of the master node available as the first
argumentof the script when it is run within the test suite.' But test.sh seems not giving such a parameter.
- contrib/buffer_capture_cmp/Makefile
"make check" does nothing when BUFFER_CAPTURE is not defined, asdescribed in itself. But I trapped by that after build
theserverby 'make CFLAGS="-DBUFFER_CAPTURE"':( It would be betterthat 'make check' without defining it prints some
message.
- buffer_capture_cmp.c
This source generates void executable when BUFFER_CAPTURE is not defined. The restriction seems to be put only to use
BUFFER_CAPTURE_FILEin bufcapt.h. If so, changing the parameter of the executable as described in the following comment
formain() would blow off the necessity for the restriction.
- buffer_capture_cmp.c/main()
The parameters for this command are the parent directories for each capture file. This is a bit incovenient for
separateuse. For example, when I want to gather the capture files from multiple servers then compare them, I should
unwillinglymake their own directories for each capture file. If no particular reason exists for the design, I suppose
itwould be more convenient that the parameters are the names of the capture files themselves.
regards,
--
Kyotaro Horiguchi
NTT Open Source Software Center
