Hi Drew,
Sorry it has taken me so long to get to this review. The comments below
are for the latest webrev you sent me in e-mail, not the one on
cr.opensolaris.org.
In general this code looks fine. I just had a few nits, listed below.
- fileset.c:
- line 278: Since you're performing a strcpy into this buffer, I don't
think you need to set path[0] = \0
- lines 279-281, 283: path has been declared on the stack. I'm not
sure if you're doing input validation elsewhere; however, it might
make sense to use strlcpy and strlcat here instead. In the case of
a long string, this would ensure that you don't exceed the buffer
space allocated on the stack.
- lines 1046 and 1047: Would it make sense to #define any of the
pickflags combinations? Do they get used in multiple locations?
- flowop_library.c:
- lines 2181 - 2183: It might also be worthwhile to use
strlcpy/strlcat or strncpy/strncat here, too.
-j
On Wed, Oct 08, 2008 at 12:36:01PM -0700, Andrew Wilson wrote:
> FileBench lovers,
>
> I just put a webrev for some changes to FileBench up on the
> opensolaris site:
>
> http://cr.opensolaris.org/~dreww/video_server/
>
> This adds two features: an example video server workload, and a set of
> file open / close / list flowops and workloads. It also fixes one small
> bug that was reported. I am hoping someone has time to do a code review
> for me.
>
> Even if you don't do a code review, feedback on any improvements I
> should make to the example Video server workload would be helpful.
>
> Drew
>
> _______________________________________________
> perf-discuss mailing list
> perf-discuss@opensolaris.org
_______________________________________________
perf-discuss mailing list
perf-discuss@opensolaris.org
