Re: [PATCH 3/3] Add a test case for the fdstream file read/write code

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 05/10/2013 11:17 AM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange@xxxxxxxxxx>
> 
> Add a test case which exercises the virFDStreamOpenFile
> and virFDStreamCreateFile methods. Ensure that both the
> synchronous and non-blocking iohelper code paths work.
> This validates the regression recently fixed which
> broke reading in non-blocking mode

Always nice to enhance our testsuite to avoid future regressions.

> 
> Signed-off-by: Daniel P. Berrange <berrange@xxxxxxxxxx>
> ---
>  .gitignore           |   1 +
>  tests/Makefile.am    |   5 +
>  tests/fdstreamtest.c | 353 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 359 insertions(+)
>  create mode 100644 tests/fdstreamtest.c

ACK with nits:

> 
> diff --git a/.gitignore b/.gitignore
> index 5e50b52..e327b37 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -145,6 +145,7 @@
>  /tests/esxutilstest
>  /tests/eventtest
>  /tests/hashtest
> +/tests/fdstreamtest

Sorting.

> +++ b/tests/Makefile.am
> @@ -107,6 +107,7 @@ test_programs = virshtest sockettest \
>          virportallocatortest \
>  	sysinfotest \
>  	virstoragetest \
> +        fdstreamtest \

Space vs. TAB inconsistency (but you can argue some of it was pre-existing).

> +
> +static int testFDStreamReadCommon(const char *scratchdir, bool blocking)
> +{

> +    if (virAsprintf(&tmpfile, "%s/input.data", scratchdir) < 0)
> +        goto cleanup;
> +
> +    if ((fd = open(tmpfile, O_CREAT|O_WRONLY, 0600)) < 0)

Worth using O_EXCL as well?

> +        reread:
> +            got = st->driver->streamRecv(st, buf + offset, want);
> +            if (got < 0) {
> +                if (got == -2 && !blocking) {
> +                    usleep(20*1000);

Spaces around *

> +static int testFDStreamWriteCommon(const char *scratchdir, bool blocking)
> +{
> +        rewrite:
> +            got = st->driver->streamSend(st, pattern + offset, want);
> +            if (got < 0) {
> +                if (got == -2 && !blocking) {
> +                    usleep(20*1000);

Spaces around *

> +
> +# define SCRATCHDIRTEMPLATE abs_builddir "/fakesysfsdir-XXXXXX"
> +
> +static int
> +mymain(void)
> +{
> +    char *scratchdir;
> +    int ret = 0;
> +    const char *iohelper = abs_builddir "/../src/libvirt_iohelper";
> +
> +    virFDStreamSetIOHelper(iohelper);
> +
> +    if (!(scratchdir = strdup(SCRATCHDIRTEMPLATE))) {
> +        fprintf(stderr, "Out of memory\n");
> +        abort();
> +    }

No need to use strdup, given our desire to switch to VIR_STRDUP.  Why
not just stack-allocate instead?

char iohelper[] = SCRATCHDIRTEMPLATE;

> +
> +    return ret==0 ? EXIT_SUCCESS : EXIT_FAILURE;

Spacing around ==

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]