Correcciones TP1

[leobellaera]

Hola, buenas noches! Antes que nada te pido mil disculpas por la demora en la corrección. Estuve con mucho laburo y temas personales y la verdad se me complicó bastante llegar a tiempo. Para mañana temprano creo ya tener las correcciones listas así podes tener el feedback que merece un trabajo de esta magnitud; voy a ir subiendo comentarios con fragmentos de tu código.

[fangrobinson]

Hola, Leo. La verdad que te re entiendo. Para mí este año fue muy muy duro, me pasaron todas.

Yo me puse a avanzar con el TP2 así esta vez logro llegar a tiempo. Si te ayuda, en el README.md traté de explicar el avance de las partes del trabajo práctico.

Para el caso de los cifradores usé pruebas internas con los casos de la práctica por lo que la incompletitud de mi tp radica principalmente en el envío de mensajes de cliente a servidor.

Tengo identificados algunos problemas. Por ejemplo: -Vigenere cifra iterando la clave desde el principio cada vez que recibe un mensaje. Debería guardar internamente la última posición de la clave para que cuando reciba un nuevo chunk a cifrar continúe desde el lugar que dejó. Esto es César no es necesario y en RC4 está funcionando bien. -Mi tda socket no funciona :( La verdad que estuve leyendo la documentación y trabajando hasta el último minuto, pero luego de la entrega preferí ponerme al día con c++ para el TP2. Quizás a vos te resulte más fácil identificar lo que no anda.

Voy a estar atento al feedback! Un abrazo

[leobellaera]

Qué tal, Robinson? Te recomiendo en lo absoluto meterle al TP1 hasta pasar todas las pruebas y dejar un poco de lado el TP2 (pensa que para el TP2 todavía tenes una reentrega asegurada y la reentrega del TP1 es el martes). Voy a mencionarte algunos detalles de tu código y, después, vamos a ver si logro encontrar las fallas en tu TDA socket

[leobellaera]

https://github.com/fangrobinson/taller-de-programacion-1-veiga-tp1-crypto-sockets/blob/0bd45e979c46ff3460eb74ed062e878662e61d7b/server_main.c#L10-L17

Estos mallocs son totalmente innecesarios! Esos strings ya los maneja en memoria el SO, por lo cual lo más simple es pasarlos como punteros (van a ser válidos en el tiempo de vida de tu programa). Además, tene un poco de cuidado cuando accedes a los elementos del array argv porque el usuario podría haber puesto menos argumentos de los pedidos (pregunta el valor de argc antes de acceder a los mismos)

[leobellaera]

https://github.com/fangrobinson/taller-de-programacion-1-veiga-tp1-crypto-sockets/blob/0bd45e979c46ff3460eb74ed062e878662e61d7b/server_main.c#L19

Un detalle menor, definí tipos mediante typedef en las definiciones de tus structs para evitar tener que usar la sintaxis struct ... y usar algo más amigable del estilo server_t sv(..)

[leobellaera]

https://github.com/fangrobinson/taller-de-programacion-1-veiga-tp1-crypto-sockets/blob/0bd45e979c46ff3460eb74ed062e878662e61d7b/server_server.c#L25

Este malloc es innecesario. Evitá el uso de memoria dinámica y aprovecha que el SO te maneja los recursos del stack, después es un bardo debuggear para encontrar bugs relacionados al correcto uso de estos recursos.

[leobellaera]

https://github.com/fangrobinson/taller-de-programacion-1-veiga-tp1-crypto-sockets/blob/0bd45e979c46ff3460eb74ed062e878662e61d7b/common_controlador_cifradores.c#L9-L18

Intenta mantener definiciones de tipos y estructuras en headers files.

[leobellaera]

https://github.com/fangrobinson/taller-de-programacion-1-veiga-tp1-crypto-sockets/blob/0bd45e979c46ff3460eb74ed062e878662e61d7b/client_main.c#L16-L26

Lo mismo que te mencioné del lado del servidor

[leobellaera]

https://github.com/fangrobinson/taller-de-programacion-1-veiga-tp1-crypto-sockets/blob/0bd45e979c46ff3460eb74ed062e878662e61d7b/client_client.c#L19-L29

De nuevo, muchos mallocs innnecesarios, aprovechá el uso del stack!

[leobellaera]

https://github.com/fangrobinson/taller-de-programacion-1-veiga-tp1-crypto-sockets/blob/0bd45e979c46ff3460eb74ed062e878662e61d7b/common_socket.c#L18-L95

La verdad no vi nada muy raro sobre tu TDA socket, pero ¿por qué no probas cambiar los unsigned short que usas para el puerto por strings? Estás haciendo una conversión que no tiene sentido porque el usuario te va a pasar los puertos en formato string. El resto de las cosas las veo realmente bien.

Si seguís sin lograr hacerlo andar, te recomiendo armar un mini programita que use tus sockets e intentar que se comuniquen entre ellos hasta que ande. Si no, otra herramienta muy copada es netcat.

[leobellaera]

Robinson, tu trabajo está realmente muy bien. Si logras hacer andar los sockets, terminar de hacer andar el cifrador Vigenere y reducir el uso de mallocs (que te los marqué en comentarios anteriores) seguramente te quede un 10. Metele con todo y cualquier duda pregunta.

[fangrobinson]

Cómo estás Leo? Te comento los cambios que hice para facilitar la corrección. Arreglé todos los typedef como me marcaste para ser consistente y más claro. Me deshice de todos los mallocs innecesarios. Ahora mismo quedaron 2: 1- para la reserva dinámica de la struct cifradora que depende del parámetro de ingreso. 2- uno para el buffer en la clase servidor y cliente.

El problema con 2 es que al ser de largo dinámico cppcheck se queja. Preferí usar malloc acá en lugar de hardcodear un 64 en esos módulos, porque siento que no es responsabilidad del módulo definir el largo del buffer, sino que lo defino en los server_main.c y client_main.c por si el día de mañana cambia el largo a ser usado.

Vigenere tenía únicamente el problema que había señalado antes y rc4 en descifrar había puesto un reinit a la estructura que me servía para unos chequeos internos que hice hace como 3 semanas.

Depuré los main de ambos cliente y servidor para ir sumando código de a poco y tratar primero de mandar un mensaje como me comentaste y logré encontrar los problemas en mi tda socket, más que nada cómo manejaba los retornos.

Durante el resto del día voy a documentar en el informe.

Un abrazo! Espero que andes mejor!

[fangrobinson]

Completé el informe con las actualizaciones que comenté como respuesta a este issue. Agregué además una descripción de cómo es la solución propuesta y las responsabilidades de cada módulo y cómo se trataron estas. Documenté los headers de los tda. Estos últimos dos cambios no creo que ameriten volver a subirlos al sercom porque para los .h la salida va a ser la misma y el informe no lo estuve subiendo en el .zip.

¡Cualquier cosa que creas que deba cambiar, avisame!

Salvo que encuentres algo, voy a pausarlo acá para retomar el TP2

Gracias de nuevo por las correcciones.

[leobellaera]

https://github.com/fangrobinson/taller-de-programacion-1-veiga-tp1-crypto-sockets/blob/4a227bc8e0549e5d05f158ca81c8555d4f923039/server_main.c#L17-L19

No queda del todo claro cómo estás obteniendo estos argumentos, podrías haber hecho una función static parse para que quede mucho más claro el parseo de entrada de datos (de todos modos es un detalle menor).

[leobellaera]

El problema con 2 es que al ser de largo dinámico cppcheck se queja. Preferí usar malloc acá en lugar de hardcodear un 64 en esos módulos, porque siento que no es responsabilidad del módulo definir el largo del buffer, sino que lo defino en los server_main.c y client_main.c por si el día de mañana cambia el largo a ser usado.

Con respecto a este comentario por qué debería definirse esta constante en el main? No tiene demasiado sentido que constantes del dominio queden expuestas "hacia afuera". De todos modos, bien por reducir bocha el uso de mallocs!

https://github.com/fangrobinson/taller-de-programacion-1-veiga-tp1-crypto-sockets/blob/4a227bc8e0549e5d05f158ca81c8555d4f923039/server_main.c#L23

https://github.com/fangrobinson/taller-de-programacion-1-veiga-tp1-crypto-sockets/blob/4a227bc8e0549e5d05f158ca81c8555d4f923039/server_server.c#L46

Si el tamaño de un chunk va a ser constante, por qué directamente no lo definís en el TDA server y te evitas usar el heap para esto?

https://github.com/fangrobinson/taller-de-programacion-1-veiga-tp1-crypto-sockets/blob/4a227bc8e0549e5d05f158ca81c8555d4f923039/client_main.c#L22

Del lado del cliente, lo mismo.

[leobellaera]

Robinson, el trabajo está muy bien en términos generales, te felicito y te deseo suerte para lo que sigue.

Nota: 9 (nueve)

[fangrobinson]

Hola! Mi razonamiento fue que si el día de mañana quisiera utilizar ese struct y sus funciones tendría que simplemente incluirlo y pasarle en tiempo de ejecución el largo del buffer, sin necesidad de recompilarlo y podría usarlo con un largo distinto cada vez o algo así. No sé por qué haría esto de cualquier modo, pero por eso decidí hacerlo de esa forma jaja. Gracias de nuevo por las correcciones!

[leobellaera]

Hola, buenas noches Robinson! El tema es, ¿cómo harías para cambiar el tamaño del buffer en tiempo de ejecución de la forma en la que lo hiciste? De todos modos no es algo grave ni mucho menos