io baseline #43
io baseline #43
Conversation
| { | ||
| if (argc != 4) | ||
| { | ||
| fprintf(stderr, "usage: put file n_data n_parities\n"); |
There was a problem hiding this comment.
use std::cerr instead of fprintf
src/io.h
Outdated
| @@ -0,0 +1,10 @@ | |||
| #ifndef __IO_H__ | |||
There was a problem hiding this comment.
__KAD_IO_H__ has less risk of conflict (IO is pretty common) and is consistent with the other guards (that uses a KAD prefix).
Also, we should add license preamble here as well.
| return SHELL_CONT; | ||
| } | ||
|
|
||
| do_put(argv[1], atoi(argv[2]), atoi(argv[3])); |
There was a problem hiding this comment.
atoi is unsafe (I think the linter may complains about it), you can use the stou32 helper from utils.cpp
src/io.cpp
Outdated
| @@ -0,0 +1,125 @@ | |||
|
|
|||
There was a problem hiding this comment.
Add license preamble:
/*
* Copyright 2017-2018 the QuadIron authors
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions are met:
*
* 1. Redistributions of source code must retain the above copyright notice,
* this list of conditions and the following disclaimer.
*
* 2. Redistributions in binary form must reproduce the above copyright notice,
* this list of conditions and the following disclaimer in the documentation
* and/or other materials provided with the distribution.
*
* 3. Neither the name of the copyright holder nor the names of its contributors
* may be used to endorse or promote products derived from this software
* without specific prior written permission.
*
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
* AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
* IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
* ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE
* LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
* CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
* SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
* INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
* CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
* ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
* POSSIBILITY OF SUCH DAMAGE.
*/
src/io.cpp
Outdated
| @@ -0,0 +1,125 @@ | |||
|
|
|||
| #include <nttec/nttec.h> | |||
There was a problem hiding this comment.
minor: the "pattern" used in the other files is
#include <stdinclude1>
#include <stdinclude2>
#include <systemdeps1>
#include <systemdeps2>
#include "local_include1"
#include "local_include2"
Here that would gives
#include <cstdio>
#include <cstdlib>
#include <streambuf>
#include <sys/mman.h>
#include <sys/stat.h>
#include <unistd.h>
#include <fcntl.h>
#include <nttec/nttec.h>
#include "io.h"
src/io.cpp
Outdated
|
|
||
| //do it | ||
| offset = 0; | ||
| for (u_int i = 0; i < fec->n_data; i++) { |
There was a problem hiding this comment.
u_int is not standard, use uint32_t instead
src/io.cpp
Outdated
|
|
||
| template class nttec::fec::RsFnt<uint32_t>; | ||
|
|
||
| int vflag = 1; |
There was a problem hiding this comment.
static int? (to limit the scope) or even static bool (since it's a flag).
src/io.cpp
Outdated
| struct stat sb; | ||
| size_t length, data_size; | ||
| off_t offset; | ||
| char cfilename[1024]; |
There was a problem hiding this comment.
use an std::string instead of a fixed size buffer?
src/io.cpp
Outdated
|
|
||
| if (fstat(fd, &sb) == -1) { | ||
| std::cerr << "fstat " << filename << " failed\n"; | ||
| return -1; |
There was a problem hiding this comment.
if we return here (or throw an exception if you follow my advice) then we will leak an open fd
src/io.cpp
Outdated
| if (length % fec->n_data != 0) { | ||
| // FIXME | ||
| std::cerr << "file size is not multiple of n_data\n"; | ||
| return -1; |
There was a problem hiding this comment.
if we return here (or throw an exception if you follow my advice) then we will leak an open fd
|
OK thorough review. Any idea why the tests are failing ? how can I check on my laptop before pushing ? |
|
The test are fine (we don't have that much for now anyway ^^") The problem are:
|
No description provided.