Skip to content

Conversation

@enerdhil
Copy link
Contributor

Server is listening and doing nothing.

@enerdhil enerdhil requested a review from Ne02ptzero April 28, 2017 15:53
continue ;
}
inet_ntop(rem_addr.ss_family, get_in_addr((struct sockaddr *)&rem_addr),
str, sizeof str);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sizeof(str)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Evidemment, my bad.

hints.ai_socktype = SOCK_STREAM; /* Use TCP stream socket */

/* Get port number as a string */
snprintf(port_str, 6, "%d", flags_get_port());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question: snprintf met un \0 à la fin de la string ?

Copy link
Contributor Author

@enerdhil enerdhil Apr 28, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Man snprintf sous debian : The functions snprintf() and vsnprintf() write at most size bytes (including the terminating null byte ('\0')) to str.

Sous OSX : The snprintf() and vsnprintf() functions will write at most size-1 of the characters printed into the output string (the size'th character then gets the terminating \0'); if the return value is greater than or equal to the size argument, the string was too short and some of the printed characters were discarded. The output is always null-terminated, unless size is 0.`

int sockfd = 0, /* Read on this one */
rem_fd = 0, /* Fd that will be attributed to the client */
rval; /* To stock getaddrinfo() return */
s8_t enable = 1; /* Int to enable the option in setsockopt */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pourquoi signed ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fondamentalement, pour rien, ca doit etre zero ou non-zero, tu préfère unsigned ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Booléen du coup, pour la clareté !

#define BACKLOG 10 /* How many pending connection we will keep in queue */

// get sockaddr, IPv4 or IPv6:
void *get_in_addr(struct sockaddr *sa)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Vérifie que le pointeur qu'on t'envoie est pas NULL avant de déréférencer. Et si la fonction est utilisée nulle part ailleurs, met la static

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep.


char str[INET6_ADDRSTRLEN];

memset(&hints, 0, sizeof hints);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sizeof(hints)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Je sais pas ce qui m'as pris sur les sizeof.

sizeof(int)) == 1)
{
/* If it fails, display the error and exit */
m_panic("Server: socket %s" , strerror(errno));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Manque un freeaddrinfo ça leak

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oublié effectivement, je patch.

- Forgot parenthesis after sizeof
- Forgot a freeaddrinfo
- Changed s8_t to u8_t
- Checked for NULL parameters in get_in_addr
- Made get_in_addr a static function
src/args.c Outdated

/**
* Path of the specifiedd PID file, if defined
* Path of the specifiedd PID file, if defined.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

specifiedd

@Ne02ptzero
Copy link
Member

Ok pour le code, j'attend le fix du build + coverage pour merge

@codecov
Copy link

codecov bot commented May 2, 2017

Codecov Report

Merging #3 into master will decrease coverage by 100%.
The diff coverage is 0%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #3     +/-   ##
=====================================
- Coverage     100%   0%   -100%     
=====================================
  Files           1    1             
  Lines          41   35      -6     
=====================================
- Hits           41    0     -41     
- Misses          0   35     +35
Impacted Files Coverage Δ
src/launch_server.c 0% <0%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f35e119...9343c61. Read the comment docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants