file browser : second command doesnt work

[botty]

I tried to develop a file browser of a connected pc. Following is the code...
main:

#include<iostream>
#include<string>
#include"socket2.h"
#include<stdio.h>
#include<time.h>
using namespace std;
char buffer[256];
bool value;
void wait ( int seconds )
{
  clock_t endwait;
  endwait = clock () + seconds * CLOCKS_PER_SEC ;
  while (clock() < endwait) {}
}
int main()
{
    int port=666;
    int choice;
    char c='y';
    string ipaddress;
    char recvmsg[256];
    char sendmsg[256];
    char recved[256];
    cout<<"1. client"<<endl ;
    cout<<"2. server"<<endl ;
    cout<<"3. exit"<<endl;
    cin>>choice;
    if(choice==3)
                 exit(0);
    else if(choice==1)
    {
         //cout<<"enter the ip address to which u want to connect"<<endl;
         //cin>>ipaddress;
         ipaddress="127.0.0.1";
         ClientSocket csocket;
         cout<<"---Attempting to connect---"<<endl;
         csocket.connecttoserver(ipaddress.c_str(),port);
         //do{
         while(1)
         {
                 cout<<"enter the string to be sent"<<endl;
                 cin.ignore();
                 cin.get(sendmsg,256);
                 csocket.SendData(sendmsg);
                 csocket.RecvFile(recved,256);
                 system("pause");
                 //cout<<"do u want to exit?y/n?"<<endl;
                 //cin>>c;
         }
         //while(c=='y');
         csocket.closeconnection();
    }
    
    else
    {
        cout<<"---Waiting for connection---";
        ServerSocket ssocket;
        ssocket.starthosting(port);
        //while(a!=1)
        //while(strcmp(recvmsg,"exit"))
        //{
        while(1)
                ssocket.RecvData(recvmsg,256);
                                     //wait(5);
        //                           }while(strcmp(recvmsg,"exit"));
        //cout<<recvmsg<<endl;
        system("pause");
    }  

}

socket2.h:

#pragma once
#include<Winsock2.h>
#include<iostream>
#include<fstream>

using namespace std;
int cnt;
bool exit_status=0;
int x=1,a=0,b=0,z=0;
class Socket
{

     protected:
               WSADATA wsadata;            
               SOCKET backup;
               SOCKET Accept;
               sockaddr_in address;
     public:
            SOCKET msocket;
            Socket();
            ~Socket();
            void closeconnection();
            bool SendData(char *);
            void RecvData(char *,int);
};
class ServerSocket: public Socket
{
      public:
             void Listen();
             void Bind(int port);
             void starthosting(int port);                 
};
class ClientSocket: public Socket
{
      public:
             void connecttoserver(const char *ipaddr, int port);
             void RecvFile(char *,int);
};
Socket::Socket()
{
                if((WSAStartup(MAKEWORD(2,2),&wsadata))!=NO_ERROR)
                {
                                                                  cout<<"error with initialization";
                                                                  system("pause");
                                                                  WSACleanup();
                                                                  exit(0);
                }
                msocket=socket(AF_INET,SOCK_STREAM,IPPROTO_TCP);
                if(msocket== INVALID_SOCKET)
                {
                            cout<<"error with initializtion 2"<<endl;
                            system("pause");
                            WSACleanup();
                            exit(0);
                }
                backup=msocket;
}
Socket::~Socket()
{
                 WSACleanup();
}
void Socket::closeconnection()
{
     WSACleanup();
     char *exit= "exit";
     SendData(exit);
     msocket=backup;
     //exit_status=1;
}
void ServerSocket::starthosting(int port)
{
     Bind(port);
     Listen();
}
void ServerSocket::Bind(int port)
{
     address.sin_family=AF_INET;
     address.sin_port=htons(port);
     address.sin_addr.s_addr= inet_addr("0.0.0.0");
     if((bind(msocket,(SOCKADDR*) &address,sizeof(address)))!=NO_ERROR)
     {
                                 cout<<"error with binding"<<endl;
                                 system("pause");
                                 WSACleanup();
                                 exit(0);
     }
}
void ServerSocket::Listen()
{
     if(listen(msocket,1)==SOCKET_ERROR)
     {
                                       cout<<"socket error"<<endl;
                                       system("pause");
                                       WSACleanup();
                                       exit(0);
     }
     Accept=accept(backup,NULL,NULL);
     while(Accept==SOCKET_ERROR)
     {
                                Accept=accept(backup,NULL,NULL);
     }
     msocket=Accept;
}
void ClientSocket::connecttoserver(const char *ipaddr,int port)
{
     address.sin_family=AF_INET;
     address.sin_addr.s_addr=inet_addr(ipaddr);
     address.sin_port=htons(port);
     if( ( cnt=connect(msocket,(SOCKADDR *) &address,sizeof( address ) ) )!=NO_ERROR)
     {
                                  cout<<"error with connection"<<endl;
                                  system("pause");
                                  WSACleanup();
                                  exit(0);
     }
     cout<<"successfully connected"<<endl;
}
bool Socket::SendData(char *buffer)
{
     send(msocket,buffer,strlen(buffer),0);
     //memset(buffer,0,sizeof(buffer));
     return TRUE;
}
void Socket::RecvData(char *buff,int size)
             {
                           //if(x==z)
                           {
                                     printf("came here");
                                     int i=recv(msocket,buff,size,0);
                                     buff[i]='\0';
                                     ++x;
                                     printf("%s",buff);
                                     {     
                                           char *line;
                                           FILE *fp=popen(buff,"r");
                                           if(fp==NULL) printf("file error");
                                           //memset(buff,0,sizeof(buff));                                           
                                           while(fgets(line,sizeof(line),fp)!='\0')
                                           {
                                                                          printf("in loop");
                                                                          SendData(line);
                                                                          
                                           }

                                           printf("out of loop");
                                           line="DONE";
                                           SendData(line);
                                           delete(line);
                                     }
                           }

                           
             }     
void ClientSocket::RecvFile(char buff[50],int size)
{
     //if(x==z)
     {
             int i=recv(msocket,buff,size,0);
             buff[i]='\0';
             ++x;
             while(strcmp(buff,"DONE"))
             //while(1)
             {
                                       i=recv(msocket,buff,size,0);
                                       buff[i]='\0';
                                       //printf("in loop recvfile");
                                       printf("%s",buff);
                                       //system("pause");
             }
     }
     //system("pause");
     memset(buff,0,sizeof(buff));
     //printf("loop exit");
}

It runs through the first cycle ( send command & get an output back). But the server side crashes in second cycle with a following error :
An Access Violation (Segmentation fault) error
at the highlighted statement (while(fgets.....) in socket2.h

[Swarvy]

The problem is probably to do with the fact you declare a char pointer but don't make it point to anything and then try and read info off it anyway.

[botty]

i didnt get your point...fgets is getting the line from the opened file & that line is assigned the "line" pointer.
Anyways, going by your point, it should not have worked in the first run...

[King Mir]

fgets(line,sizeof(line),fp)

This only works if line is a stack array, not a pointer.

You need to allocate space for fgets to write to, such as by making line an array or by calling malloc(). If you use malloc, you will need to specify the length to fgets explicitly (instead of sizeof(line) and call free() later.

The fact that your program sometimes works does not mean that there isn't anything wrong. You just got lucky. Or unlucky depending on how you look at it.

[Elysia]

This is C++, so using malloc is usually a bad idea.
You should start making this C into C++.
Replace char with std::string. If you need a buffer, then use std::vector<char>.
Replace fgets with std::getline.
Replace printf with std::cout.
Replace memset with std::fill.

[King Mir]

Elysia's right.

Also you should only be putting the class definitions is the header file. All those member functions and constructors should go in a separate source file.