Програмиране с Rust

Курс във Факултета по Математика и Информатика към СУ

Решение на Network Packets от Петър Петров

Обратно към всички решения

Към профила на Петър Петров

Резултати

  • 13 точки от тестове
  • 0 бонус точки
  • 13 точки общо
  • 10 успешни тест(а)
  • 5 неуспешни тест(а)

Код

use std::fmt;
use std::str;
#[derive(Debug)]
pub enum PacketError {
InvalidPacket,
InvalidChecksum,
UnknownProtocolVersion,
CorruptedMessage,
}
impl fmt::Display for PacketError {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
write!(f,"{:?}",self)
}
}
impl std::error::Error for PacketError {}
pub fn checksum(s:&[u8]) -> u32{
let mut sum:u32=0;
for &x in s{
sum+=x as u32;
}
sum
}
#[derive(PartialEq, Debug)]
pub struct Packet<'a> {
s: &'a [u8],
n: usize
}

Това не са много удачни имена на променливи. Еднобуквени променливи в рамките на един кратък closure може да са удобни, примерно some_iterator.sum(|n| *n as u32), но когато тези полета са част от състоянието на структура и се преизползват на много места, по-добре е да бъдат малко по-описателни. Ако някой чете кода и срещне някъде из методите self.s, ще има нужда да се консултира до конструктора, докато self.source може би ще е по-лесно за разбиране и запомняне.

Андрей
impl<'a> Packet<'a> {
pub fn from_source(source: &'a[u8], size: u8) -> (Self, &'a [u8]) {
let sizeToUsize: usize=size as usize;
let lengthOfArray: usize=source.len() as usize;
if sizeToUsize >= lengthOfArray{
return (Packet{s:&source,n:lengthOfArray},b"")
}
else if sizeToUsize == 0 {
panic!();
}
else {
(Packet{s:&source[..sizeToUsize],n:sizeToUsize},&source[sizeToUsize..])
}
}
pub fn payload(&self) -> &[u8] {
&self.s[..self.n]
}

Доколкото виждам по-горе, ти вече съхраняваш в self.s slice с нужния размер, примерно s: &source[..sizeToUsize]. Не мисля, че има смисъл тук да подаваш размера отново -- можеш просто да върнеш &self.s.

Андрей
pub fn serialize(&self) -> Vec<u8> {
let mut vec : Vec<u8>= Vec::new();
vec.push(1 as u8);
vec.push(self.n as u8);
let mut sum:u32=0;
for &x in self.s{
vec.push(x);
sum+=x as u32;
}
let b1 : u8 = ((sum >> 24) & 0xff) as u8;
vec.push(b1);
let b2 : u8 = ((sum >> 16) & 0xff) as u8;
vec.push(b2);
let b3 : u8 = ((sum >> 8) & 0xff) as u8;
vec.push(b3);
let b4 : u8 = (sum & 0xff) as u8;
vec.push(b4);
vec
}
pub fn deserialize(bytes: &[u8]) -> Result<(Packet, &[u8]), PacketError> {
let size :usize=bytes.len();
if bytes[1] > (size-6) as u8 {
return Err(PacketError::InvalidPacket);
}
if bytes[0] != 1 {
return Err(PacketError::UnknownProtocolVersion);
}
let sizeOfPacket=bytes[1] as usize;
let mut sum:u32=checksum(&bytes[2..sizeOfPacket+2]);
let b1 : u8 = ((sum >> 24) & 0xff) as u8;
let b2 : u8 = ((sum >> 16) & 0xff) as u8;
let b3 : u8 = ((sum >> 8) & 0xff) as u8;
let b4 : u8 = (sum & 0xff) as u8;
if bytes[sizeOfPacket+2] != b1 || bytes[sizeOfPacket+3] != b2 || bytes[sizeOfPacket+4] != b3 || bytes[sizeOfPacket+5] != b4 {
return Err(PacketError::InvalidChecksum);
}
Ok((Packet{s:&bytes[2..sizeOfPacket+2],n:sizeOfPacket},&bytes[sizeOfPacket+6..]))
}
}
pub struct PacketSerializer<'a> {
vec: Vec<Packet<'a>>
}
impl<'a> Iterator for PacketSerializer<'a> {
type Item = Packet<'a>;
fn next(&mut self) -> Option<Self::Item> {
if self.vec.len() == 0 {
return None
}
self.vec.pop()

Метода pop вече връща None ако няма какво да pop-не, така че горната проверка за дължина е напълно ненужна.

От друга страна, това е и грешна логика, понеже пълниш вектора с push-ване и го празниш с pop-ване, което обръща реда на пакетите.

Андрей
}
}
pub trait Packetable: Sized {
fn to_packets(&self, packet_size: u8) -> PacketSerializer;
fn to_packet_data(&self, packet_size: u8) -> Vec<u8>;
fn from_packet_data(packet_data: &[u8]) -> Result<Self, PacketError>;
}
impl Packetable for String {
fn to_packets(&self, packet_size: u8) -> PacketSerializer {
let mut vec:Vec<Packet>=Vec::new();
let mut len =self.len();
let mut count=0;
let packet_size_to_usize = packet_size as usize;
while len >= packet_size_to_usize {
let (packet, remainder) = Packet::from_source(&self[count..count+packet_size_to_usize+1].as_bytes(), packet_size);
vec.push(packet);
len=len-packet_size_to_usize;
count=count+packet_size_to_usize;
}
if len > 0 {
let (packet, remainder) = Packet::from_source(&self[count..count+len].as_bytes(), len as u8);
vec.push(packet);
}
(PacketSerializer{vec:vec})
}
fn to_packet_data(&self, packet_size: u8) -> Vec<u8> {
let mut vec:Vec<u8>=Vec::new();
let mut len =self.len();
let mut count=0;
let packet_size_to_usize = packet_size as usize;
let mut arr=Packetable::to_packets(self,packet_size);
loop {
let x=arr.next();
if x == None {
break;
}
match x {
Some(y) => vec.extend(&y.serialize()) ,
None => println!("Fail"),
}
}
vec
}
fn from_packet_data(packet_data: &[u8]) -> Result<Self, PacketError> {
let mut vec:Vec<u8>=Vec::new();
if packet_data.len() >= 6 {
let mut end=packet_data[1] as usize+6;
let mut begin=0;
while end <=packet_data.len() {
let (packet,arr)=Packet::deserialize(&packet_data[begin..end]).unwrap();
vec.extend(packet.payload());
begin=end;
if end+1 >= packet_data.len(){ break;}
end=(packet_data[begin+1]as usize)+end+6;
}
let message = String::from_utf8(vec);
let message = match message {
Ok(value) => return Ok(value),
Err(e) => return Err(PacketError::CorruptedMessage),
};
}
else{
return Err(PacketError::InvalidPacket)
}
}
}

Лог от изпълнението

Compiling solution v0.1.0 (/tmp/d20200111-2173579-1v53rij/solution)
warning: unused import: `std::str`
 --> src/lib.rs:2:5
  |
2 | use std::str;
  |     ^^^^^^^^
  |
  = note: `#[warn(unused_imports)]` on by default

warning: unused variable: `remainder`
   --> src/lib.rs:130:26
    |
130 |             let (packet, remainder) = Packet::from_source(&self[count..count+packet_size_to_usize+1].as_bytes(), packet_size);
    |                          ^^^^^^^^^ help: consider prefixing with an underscore: `_remainder`
    |
    = note: `#[warn(unused_variables)]` on by default

warning: unused variable: `remainder`
   --> src/lib.rs:137:26
    |
137 |             let (packet, remainder) = Packet::from_source(&self[count..count+len].as_bytes(), len as u8);
    |                          ^^^^^^^^^ help: consider prefixing with an underscore: `_remainder`

warning: unused variable: `len`
   --> src/lib.rs:146:17
    |
146 |         let mut len =self.len();
    |                 ^^^ help: consider prefixing with an underscore: `_len`

warning: unused variable: `count`
   --> src/lib.rs:147:17
    |
147 |         let mut count=0;
    |                 ^^^^^ help: consider prefixing with an underscore: `_count`

warning: unused variable: `packet_size_to_usize`
   --> src/lib.rs:148:13
    |
148 |         let packet_size_to_usize = packet_size as usize;
    |             ^^^^^^^^^^^^^^^^^^^^ help: consider prefixing with an underscore: `_packet_size_to_usize`

warning: unused variable: `arr`
   --> src/lib.rs:170:29
    |
170 |                 let (packet,arr)=Packet::deserialize(&packet_data[begin..end]).unwrap();
    |                             ^^^ help: consider prefixing with an underscore: `_arr`

warning: unused variable: `message`
   --> src/lib.rs:178:17
    |
178 |             let message = match message {
    |                 ^^^^^^^ help: consider prefixing with an underscore: `_message`

warning: unused variable: `e`
   --> src/lib.rs:180:21
    |
180 |                 Err(e) => return Err(PacketError::CorruptedMessage),
    |                     ^ help: consider prefixing with an underscore: `_e`

warning: variable does not need to be mutable
  --> src/lib.rs:87:13
   |
87 |         let mut sum:u32=checksum(&bytes[2..sizeOfPacket+2]);
   |             ----^^^
   |             |
   |             help: remove this `mut`
   |
   = note: `#[warn(unused_mut)]` on by default

warning: variable does not need to be mutable
   --> src/lib.rs:146:13
    |
146 |         let mut len =self.len();
    |             ----^^^
    |             |
    |             help: remove this `mut`

warning: variable does not need to be mutable
   --> src/lib.rs:147:13
    |
147 |         let mut count=0;
    |             ----^^^^^
    |             |
    |             help: remove this `mut`

warning: variable `sizeToUsize` should have a snake case name
  --> src/lib.rs:38:13
   |
38 |         let sizeToUsize: usize=size as usize;
   |             ^^^^^^^^^^^ help: convert the identifier to snake case: `size_to_usize`
   |
   = note: `#[warn(non_snake_case)]` on by default

warning: variable `lengthOfArray` should have a snake case name
  --> src/lib.rs:39:13
   |
39 |         let lengthOfArray: usize=source.len() as usize;
   |             ^^^^^^^^^^^^^ help: convert the identifier to snake case: `length_of_array`

warning: variable `sizeOfPacket` should have a snake case name
  --> src/lib.rs:86:13
   |
86 |         let sizeOfPacket=bytes[1] as usize;
   |             ^^^^^^^^^^^^ help: convert the identifier to snake case: `size_of_packet`

warning: unused import: `std::str`
 --> src/lib.rs:2:5
  |
2 | use std::str;
  |     ^^^^^^^^
  |
  = note: `#[warn(unused_imports)]` on by default

warning: unused variable: `remainder`
   --> src/lib.rs:130:26
    |
130 |             let (packet, remainder) = Packet::from_source(&self[count..count+packet_size_to_usize+1].as_bytes(), packet_size);
    |                          ^^^^^^^^^ help: consider prefixing with an underscore: `_remainder`
    |
    = note: `#[warn(unused_variables)]` on by default

warning: unused variable: `remainder`
   --> src/lib.rs:137:26
    |
137 |             let (packet, remainder) = Packet::from_source(&self[count..count+len].as_bytes(), len as u8);
    |                          ^^^^^^^^^ help: consider prefixing with an underscore: `_remainder`

warning: unused variable: `len`
   --> src/lib.rs:146:17
    |
146 |         let mut len =self.len();
    |                 ^^^ help: consider prefixing with an underscore: `_len`

warning: unused variable: `count`
   --> src/lib.rs:147:17
    |
147 |         let mut count=0;
    |                 ^^^^^ help: consider prefixing with an underscore: `_count`

warning: unused variable: `packet_size_to_usize`
   --> src/lib.rs:148:13
    |
148 |         let packet_size_to_usize = packet_size as usize;
    |             ^^^^^^^^^^^^^^^^^^^^ help: consider prefixing with an underscore: `_packet_size_to_usize`

warning: unused variable: `arr`
   --> src/lib.rs:170:29
    |
170 |                 let (packet,arr)=Packet::deserialize(&packet_data[begin..end]).unwrap();
    |                             ^^^ help: consider prefixing with an underscore: `_arr`

warning: unused variable: `message`
   --> src/lib.rs:178:17
    |
178 |             let message = match message {
    |                 ^^^^^^^ help: consider prefixing with an underscore: `_message`

warning: unused variable: `e`
   --> src/lib.rs:180:21
    |
180 |                 Err(e) => return Err(PacketError::CorruptedMessage),
    |                     ^ help: consider prefixing with an underscore: `_e`

warning: variable does not need to be mutable
  --> src/lib.rs:87:13
   |
87 |         let mut sum:u32=checksum(&bytes[2..sizeOfPacket+2]);
   |             ----^^^
   |             |
   |             help: remove this `mut`
   |
   = note: `#[warn(unused_mut)]` on by default

warning: variable does not need to be mutable
   --> src/lib.rs:146:13
    |
146 |         let mut len =self.len();
    |             ----^^^
    |             |
    |             help: remove this `mut`

warning: variable does not need to be mutable
   --> src/lib.rs:147:13
    |
147 |         let mut count=0;
    |             ----^^^^^
    |             |
    |             help: remove this `mut`

warning: variable `sizeToUsize` should have a snake case name
  --> src/lib.rs:38:13
   |
38 |         let sizeToUsize: usize=size as usize;
   |             ^^^^^^^^^^^ help: convert the identifier to snake case: `size_to_usize`
   |
   = note: `#[warn(non_snake_case)]` on by default

warning: variable `lengthOfArray` should have a snake case name
  --> src/lib.rs:39:13
   |
39 |         let lengthOfArray: usize=source.len() as usize;
   |             ^^^^^^^^^^^^^ help: convert the identifier to snake case: `length_of_array`

warning: variable `sizeOfPacket` should have a snake case name
  --> src/lib.rs:86:13
   |
86 |         let sizeOfPacket=bytes[1] as usize;
   |             ^^^^^^^^^^^^ help: convert the identifier to snake case: `size_of_packet`

    Finished test [unoptimized + debuginfo] target(s) in 3.69s
     Running target/debug/deps/solution-a73e64ec87929bd0

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out

     Running target/debug/deps/solution_test-38971695424b36d5

running 15 tests
test solution_test::test_construct_packet_from_unicode ... ok
test solution_test::test_construct_packet_no_remainder ... ok
test solution_test::test_construct_packet_with_remainder ... ok
test solution_test::test_construct_packet_with_remainder_cyrillic ... ok
test solution_test::test_consuming_packets ... thread '<unnamed>' panicked at 'assertion failed: `(left == right)`
  left: `"bazbar foo "`,
 right: `"foo bar baz"`', tests/solution_test.rs:204:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace.
FAILED
test solution_test::test_deserialize_invalid_packet ... ok
test solution_test::test_deserialize_packet ... ok
test solution_test::test_deserialize_unicode_packet ... ok
test solution_test::test_full_roundtrip ... thread '<unnamed>' panicked at 'assertion failed: `(left == right)`
  left: `"foo bar baz"`,
 right: `"zfoo bar ba"`', tests/solution_test.rs:215:9
FAILED
test solution_test::test_full_roundtrip_for_zero_size_string ... thread '<unnamed>' panicked at 'called `Result::unwrap()` on an `Err` value: InvalidPacket', src/libcore/result.rs:1165:5
FAILED
test solution_test::test_invalid_packet_combination ... thread '<unnamed>' panicked at 'byte index 3 is not a char boundary; it is inside 'ю' (bytes 2..4) of `сюрприз`', src/libcore/str/mod.rs:2068:5
FAILED
test solution_test::test_iterating_packets ... thread '<unnamed>' panicked at 'assertion failed: `(left == right)`
  left: `[98, 97, 122]`,
 right: `[102, 111, 111, 32]`', tests/solution_test.rs:178:9
FAILED
test solution_test::test_iterating_packets_for_zero_size_string ... ok
test solution_test::test_serialize_packet ... ok
test solution_test::test_zero_size ... ok

failures:

---- solution_test::test_consuming_packets stdout ----
thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `"bazbar foo "`,
 right: `"foo bar baz"`', tests/solution_test.rs:197:5

---- solution_test::test_full_roundtrip stdout ----
thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `"foo bar baz"`,
 right: `"zfoo bar ba"`', tests/solution_test.rs:210:5

---- solution_test::test_full_roundtrip_for_zero_size_string stdout ----
thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: InvalidPacket', tests/solution_test.rs:221:5

---- solution_test::test_invalid_packet_combination stdout ----
thread 'main' panicked at 'byte index 3 is not a char boundary; it is inside 'ю' (bytes 2..4) of `сюрприз`', tests/solution_test.rs:232:5

---- solution_test::test_iterating_packets stdout ----
thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `[98, 97, 122]`,
 right: `[102, 111, 111, 32]`', tests/solution_test.rs:172:5


failures:
    solution_test::test_consuming_packets
    solution_test::test_full_roundtrip
    solution_test::test_full_roundtrip_for_zero_size_string
    solution_test::test_invalid_packet_combination
    solution_test::test_iterating_packets

test result: FAILED. 10 passed; 5 failed; 0 ignored; 0 measured; 0 filtered out

error: test failed, to rerun pass '--test solution_test'

История (1 версия и 8 коментара)

Петър качи първо решение на 02.12.2019 21:39 (преди почти 6 години)

Кода е доста sloppy. Имената на променливи като vec, x, n, s, могат да бъдат доста по-ясни и четими. Защо "vec" а не "packets" или "result" или дори "storage"? Някакво име, което да описва каква е целта на тази променлива, а не какъв е типа ѝ? Защо "x" или "y", а не "packet"?

Има места и на които ти се губят някои базови Rust-ки конструкции, примерно, учудвам се, че си използвал loop за нещо, което директно може да се итерира. Може би ти се губят някои лекции? Добре ще е да ги препрочетеш и/или да разгледаш останалите решения за идеи как можеш да си подобриш кода.

Силно те съветвам и да минеш през warning-ите на компилатора -- има доста проблеми като неизползвани променливи и camelCased имена, които лесно се оправят, и които могат да ти докарат кода доста по близо до стандартен Rust. Можеш да използваш и rustfmt за да си изчистиш неща като неконсистентен spacing.

Що се отнася до fail-ващите тестове -- едното нещо, което намерих е че използваш push и pop, но това работи като стек. добавяш елементи в края, и ги премахваш от края, това означава, че пакетите се обръщат. Можеш вместо pop да ползваш remove(0), макар че вероятно има и по-удобни начини да напишеш итератора -- виж моето решение например. Другите проблеми не можах да ги дебъгна, но си мисля, че ако опростиш кода, ще ги намериш и сам.